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

[Lionel Debroux]

Alexandre Julliard wrote:
> [e-mail] 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.