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

[Lionel Debroux]

Alexandre Julliard wrote:
> lionel_debroux(a)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.