[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