kernel32: Implement most of CopyFileEx

Andrey Turkin andrey.turkin at gmail.com
Tue May 8 01:39:48 CDT 2007


Sorry for jumping in, but I have some comments as well.
Dan, CopyFileA does not leaks a string (note BOOL flags). Instead first 
call use static TEB-based buffer so this is OK, given that recursion is 
impossible in this case.
Kevin, you are setting file size before copy starts. I'm not sure 
Windows does so; also how would that work if destination file system 
does not support sparse files?

Dan Kegel wrote:
> Nice to see somebody filling in the gaps like this.
>
> A few nits based on a superficial reading:
>
> CopyFileA leaks a string.  (I know, it did before your change, too.)
>
> copy_file_open_dest's interface comment has the wrong name.
>
> +    for(i = 0; i < sizeof(flags) / sizeof(flags[0]); i++)
> +    {
> +        if(for_write) {
>
> Please use the same whitespace conventions as in the
> rest of the file (in particular, leave a space after keywords like
> for and if).
>
> +    if ((h1 = copy_file_open_source(source, copyFlags &
> COPY_FILE_OPEN_SOURCE_FOR_WRITE)) == INVALID_HANDLE_VALUE)
>
> That line's too long... try to keep them under 80 chars.
>
> +    // FIXME: total_file_size should include the sum of all streams
>
> Use C comments, not C++ comments.
>
> +        DWORD result = progressRoutine(total_file_size,
> total_bytes_transferred, stream_size, stream_bytes_transferred,
> +                                       stream_number,
> CALLBACK_STREAM_SWITCH, h1, h2, appData);
>
> Another instance of line longer than 80 chars.  (You have several.)
>
> You don't have a conformance test, nor was there one for CopyFile.
> Maybe you should consider writing one, and making sure it passes
> both on Wine and Windows.
>
> Say, what app is this for?
> - Dan
>
>
>




More information about the wine-devel mailing list