Oops, I must have hit the wrong reply button.
>>> The "right" way would probably to do the copying yourself by
>>> read/write.. but I dunno.
>> Except that it would ignore the permissions issues that have already
>> been coded into the copy routines (and any updates that may eventually
> No, CreateFile (and friends) does the permissions checks (which you
> would still have to call).
That was worded poorly, Copy/Move already handle copying file attributes and I imagine would eventually implement copying the access control list information. Implementing ReplaceFile as calls to either Copy or Move takes these issues into account.
>> be made to these routines). Also, it seems to me that MSDN is
>> describing the list of function calls that ReplaceFile actually makes.
> No it doesn't?
From my perspective the "remarks" section discusses the implementation of ReplaceFile as calls to existing API functions (as a sort of macro) as a convenience. Granted, it discusses moving files instead of copying but that conflicted with the file locking.
Should I make a new patch file with the previously attached changes or would you prefer that the function be implemented in the read/write manner? Re-implementing the function in such a fashion would make it possible to do a more move-like operation while maintaining the file locking, but would lose out on any future improvements to CopyFile.
Erich Hoover
ehoover@mines.edu
On 2/28/07, Felix Nawothnig <flexo@holycrap.org
> wrote:(CC-ing wine-devel again)
Erich Hoover wrote:
>> The "right" way would probably to do the copying yourself by
>> read/write.. but I dunno.
> Except that it would ignore the permissions issues that have already
> been coded into the copy routines (and any updates that may eventually
No, CreateFile (and friends) does the permissions checks (which you
would still have to call).
> be made to these routines). Also, it seems to me that MSDN is
> describing the list of function calls that ReplaceFile actually makes.
No it doesn't?
Your other objections were right. Your last version follows for wine-devel.
> ------------------------------------------------------------------------
>
> /**************************************************************************
> * ReplaceFileW (KERNEL32.@)
> * ReplaceFile (KERNEL32.@)
> */
> BOOL WINAPI ReplaceFileW(LPCWSTR lpReplacedFileName,LPCWSTR lpReplacementFileName,
> LPCWSTR lpBackupFileName, DWORD dwReplaceFlags,
> LPVOID lpExclude, LPVOID lpReserved)
> {
> HANDLE hReplaced, hReplacement;
> BOOL skipBackup = FALSE;
>
> if(dwReplaceFlags)
> FIXME("Ignoring flags %x\n", dwReplaceFlags);
> /* First two arguments are mandatory */
> if(!lpReplacedFileName || !lpReplacementFileName)
> {
> SetLastError( ERROR_INVALID_PARAMETER );
> return FALSE;
> }
> /*
> * Lock the "replacement" file, fail if it does not exist
> */
> if((hReplacement = CreateFileW(lpReplacementFileName, GENERIC_READ,
> FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
> NULL, OPEN_EXISTING, 0, 0)) == INVALID_HANDLE_VALUE)
> {
> return FALSE;
> }
> if(!LockFile( hReplacement, 0, 0, 0, 0 ))
> {
> CloseHandle( hReplacement );
> return FALSE;
> }
> /*
> * Lock the "replaced" file while we're working.
> */
> if((hReplaced = CreateFileW(lpReplacedFileName, GENERIC_READ,
> FILE_SHARE_READ | FILE_SHARE_WRITE,
> NULL, OPEN_EXISTING, 0, 0)) == INVALID_HANDLE_VALUE)
> {
> /* If "replaced" does not exist then create it for the lock, but skip backup */
> if((hReplaced = CreateFileW(lpReplacedFileName, GENERIC_READ,
> FILE_SHARE_READ | FILE_SHARE_WRITE,
> NULL, CREATE_ALWAYS, 0, 0)) == INVALID_HANDLE_VALUE)
> {
> UnlockFile( hReplacement, 0, 0, 0, 0 );
> CloseHandle( hReplacement );
> return FALSE;
> }
> skipBackup = TRUE;
> }
> if(!LockFile( hReplaced, 0, 0, 0, 0 ))
> {
> UnlockFile( hReplacement, 0, 0, 0, 0 );
> CloseHandle( hReplacement );
> CloseHandle( hReplaced );
> return FALSE;
> }
> /*
> * If the user wants a backup then that needs to be performed first
> */
> if( lpBackupFileName && !skipBackup )
> {
> /* If an existing backup exists then copy over it */
> if(!CopyFileW( lpReplacedFileName, lpBackupFileName, FALSE ))
> goto replace_fail;
> }
> /*
> * Now that the backup has been performed (if requested), copy the replacement
> * into place
> */
> if(!CopyFileW( lpReplacementFileName, lpReplacedFileName, FALSE ))
> {
> /* Assume that all access denied errors are a write problem. */
> if(GetLastError() == ERROR_ACCESS_DENIED)
> SetLastError( ERROR_UNABLE_TO_REMOVE_REPLACED );
> else
> SetLastError( ERROR_UNABLE_TO_MOVE_REPLACEMENT );
> goto replace_fail;
> }
> /* Delete the replacement file */
> if(!DeleteFileW( lpReplacementFileName ))
> return FALSE;
> /* Unlock the handles */
> UnlockFile( hReplacement, 0, 0, 0, 0 );
> CloseHandle( hReplacement );
> UnlockFile( hReplaced, 0, 0, 0, 0 );
> CloseHandle( hReplaced );
> /* All done, file replacement successful */
> return TRUE;
>
> replace_fail:
> UnlockFile( hReplacement, 0, 0, 0, 0 );
> CloseHandle( hReplacement );
> UnlockFile( hReplaced, 0, 0, 0, 0 );
> CloseHandle( hReplaced );
> return FALSE;
> }