Re: advpack: Implement ExtractFilesW [try 2]

[James Hawkins]

2009/6/11 Vladimir Pankratov :
> Hello all.
>
> Implemented ExtractFilesW
>
> Changed files:
>  advpack/files.c

+    if (GetFileAttributesA(ExpandDir) == INVALID_FILE_ATTRIBUTES)
+        return HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND);

Why are you testing this in ExtractFilesA?  Almost all of the logic
should be passed on to ExtractFilesW so we don't duplicate code.

+    if (FileList != NULL)

Please don't change the style of the file.  if (FileList) is fine.

+        WideCharToMultiByte( CP_ACP, 0, FileList, -1, szFileList,
sizeof(szFileList), NULL, NULL );
+        szConvertedList = convert_file_list(szFileList, &dwFileCount);

I'm not sure this is the way to go.  You're converting A -> W -> A.
This happens because cabinet.Extract is (as far as we know) ansi-only.
 In this case, I think it's ok to make ExtractFilesW call into
ExtractFilesA, with an appropriate comment as to why this is
happening.

advpack: Implement ExtractFilesW [try 2]

[Detlef Riekenberg]

On Do, 2009-06-11 at 15:04 -0700, James Hawkins wrote:
> 2009/6/11 Vladimir Pankratov :
> > Hello all.
> >
> > Implemented ExtractFilesW
> >
> > Changed files:
> >  advpack/files.c
> You're converting A -> W -> A.
> This happens because cabinet.Extract is (as far as we know) ansi-only.
>  In this case, I think it's ok to make ExtractFilesW call into
> ExtractFilesA, with an appropriate comment as to why this is
> happening.

The crosscall should be avoided, when possible.
As example, move the implementation to extract_filesAW(..., BOOL
is_unicode)
and made the API entry points a wrapper around this.