shell32: for FO_COPY, if the destination is an empty string, SHFileOperation should use the current directory.

Lionel Debroux lionel_debroux at yahoo.fr
Thu May 21 07:21:37 CDT 2009


Alexandre Julliard wrote:
> lionel_debroux at yahoo.fr writes:
> 
>> @@ -1179,6 +1181,15 @@ static HRESULT copy_files(FILE_OPERATION *op, const FILE_LIST *flFrom, FILE_LIST
>>      const FILE_ENTRY *entryToCopy;
>>      const FILE_ENTRY *fileDest = &flTo->feFiles[0];
>>  
>> +    /* If the destination is empty, SHFileOperation should use the current directory */
>> +    if (!fileDest)
>> +    {
>> +        WCHAR currd[MAX_PATH];
>> +        GetCurrentDirectoryW(MAX_PATH, currd);
>> +        parse_file_list(flTo, currd);
>> +        fileDest = &flTo->feFiles[0];
>> +    }
> 
> This can't possibly work. parse_file_list requires a null-terminated
> list, and you will be overwriting the previous data.
Well, AFAICS, there is no previous data in flTo, because of the early 
abort upon empty path in parse_file_list():
     if (!szFiles[0])
         return ERROR_ACCESS_DENIED;

right before

     flList->feFiles = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
                                 flList->num_alloc*sizeof(FILE_ENTRY));

I checked it by adding a trace to the early exit of destroy_file_list():
     if (!flList || !flList->feFiles)
and calling destroy_file_list() before the call to parse_file_list() in 
copy_files() added by the patch.


I've updated the patch to add more comments and to check whether the 
destination file name is empty, instead of checking flTo->feFiles 
against NULL, though that should be equivalent due to
     ZeroMemory(&flTo, sizeof(FILE_LIST));
in SHFileOperationW.

Is this try2 closer to the correct way of doing things ?


Lionel.


More information about the wine-devel mailing list