xcopy try 2

Detlef Riekenberg wine.dev at web.de
Fri Feb 23 18:00:06 CST 2007


On Fr, 2007-02-23 at 16:44 +0000, Ann & Jason Edmeades wrote:

Looks nice, but a view over the code result in some more comments:

--- a/configure.ac
+++ b/configure.ac
Not needed, when sending the Patch
(and that line causes already an offset, when patching wine)
"tools/make_makefiles" takes care of all required changes and
when Alexandre commit your Patch, the changes from
make_makefiles and autoconf are included in the commit.

+EXTRADEFS = -DUNICODE
There is no need for that define. 
Remove that line to get a warning for all ANSI-Functions, that
missed the "W"


+C_SRCS = \
+        xcopy.c

Spaces are wrong before the filename. You must use TAB in makefiles



+/* Prototypes */
This can be avoided completly, when you rearange your functions
(so main() is then the last function in the file)


+   printf("Invalid number of parameters - Use xcopy /? for help\n");

The text should be read from the resources, so it can be localized .
You should convert the UNICODE-Message with WideCharToMultiByte.
When the output-handle is the console, printing UNICODE-messages
avoid converting the message twice (xcopy and kernel32) 

> +        if (*argvW[0] != '/') {
> +            if (suppliedsource[0] == 0x00) {
> +                lstrcpyW(suppliedsource, *argvW);
> +            } else {
> +                lstrcpyW(supplieddestination, *argvW);
> +            }

This is wrong. tested with: "xcopy file1 file2 file3"
Your code would copy file1 to file3, while w2k fails with
"Unzulässige Parameteranzahl" (Invalid number of parameters)

I suggest to initialize supplieddestination as empty string
and copy the arg to supplieddestination only, when it is emtpy.
When supplieddestination was not empty, quit with the error.

You need to fill supplieddestination with the default "."
before     /* Trace out the supplied information */,
when it is still empty at that location.


+    if (flags & OPT_SIMULATE) {
+        printf("%d file(s) would be copied\n", filesCopied);
+    } else {
+        printf("%d file(s) copied\n", filesCopied);
+    }

As Above: Format-Pattern from the resource and WriteFile() 


+    if (GetFullPathName(suppliedsource, MAX_PATH, 

"W" missing: => GetFullPathNameW
(More Functions in various locations and 
WIN32_FIND_DATA => WIN32_FIND_DATAW)


I did not test the code very much, but I think, you did.

(ToDo-Reminder:
  "xcopy file1 name_not_found" => 
  is "name_not_found" a file or a directory)


Thanks for your work. It's pretty good as start!

 
-- 
 
By by ... Detlef





More information about the wine-devel mailing list