[PATCH 0/5] (4th attempt) CMD command line parsing

Frédéric Delanoy frederic.delanoy at gmail.com
Thu Sep 29 03:54:33 CDT 2011


Hi Martin,

I reviewed your patches and have found some (minor) issues:

* Your patches email line subjects should have '(try N') appended. I
*believe* this is needed for the project leader patches handling
scripts
You can just sed the "git format-patch" generated patches for instance

* PATCH 1/5
 create mode 100755 programs/cmd/tests/test_cmdline.cmd
 create mode 100755 programs/cmd/tests/test_cmdline.cmd.exp

These files are added as executable. They should be marked as
non-executable since they contain/can potentially contain @keyword at s
(e.g. @space@, @todo_wine@ which won't be recognized by a native cmd,
but need to be processed beforehand).
While one might argue that test_cmdline.cmd is executable,
test_cmdline.cmd.exp definitely isn't.
(this may be due to the fact the files were created on windows)

* PATCH 2/5
-  args  = argc;
+  cmdline = WCMD_strdupW(GetCommandLineW());
+  if (cmdline == NULL) {
+      SetLastError(ERROR_OUTOFMEMORY);
+      exit(1);
+  }

Using SetLastError before exit(1) is a bit pointless IMHO. You'd
better use something like WINE_ERR("Could not allocate memory\n");

* PATCH 5/5
The WCMD_parameter doc should remain in front on WCMD_parameter function

That's all. Great work!

I've also had some weird test failures running "make testclean && make
test" for patch 2
batch.c:302: Test succeeded inside todo block: unexpected char 0x0
position -1 in line 18 (got '0 ', wanted '0 at space@')
batch.c:302: Test failed: unexpected char 0x65 position 0 in line 19
(got 'error 9009', wanted '1 at space@')
[and some additional messages]
Those are probably due to an issue with the testrunner, since the line
numbers aren't correct, and the test_cmdline.cmd works nicely when
test_builtins.* files are removed.
I'll look into that .

Frédéric



More information about the wine-devel mailing list