Re: MBCS minor bug fix

[Juan Lang]

Hi Hirofumi,

the subject of your patch doesn't tell us very much.  Most submissions
are in fact bug fixes in fact, so please describe what the bug is at
least.  A test case showing what's wrong with the existing code, and
that shows that your patch fixes it, would be much better.

Furthermore,
-  if (!lpszPath || (iLen = strlen(lpszPath)) >= MAX_PATH)
-    return NULL;
+  while(*pch != '\0')
+  {
+      pchLast = pch;
+      pch = CharNext(pch);
+  }

Here you're discarded a test of lpszPath against NULL.  That can't be
correct, since the existing tests show that Windows returns NULL in
this case.  (See
http://source.winehq.org/source/dlls/shlwapi/tests/path.c#L620 ).  You
must run 'make test' in the tests directory of the code you're
changing (at least), and the tests must pass, in order for your
changes to be acceptable.

Also, the call to CharNext isn't allowed in Wine's source.  (Did you
try to compile it?  It should have failed to compile.)  You must use
either CharNextA or CharNextW, whichever is appropriate, instead.

+  if(MAX_PATH <= lstrlen(lpszPath))
+      return NULL;
This change seems like an unnecessary style change.  The existing test
of lpszPath and its length accomplished the same thing, so please
leave them alone and concentrate on the fix itself.

Finally, if I understand the intent of your patch correctly, the
changes to PathAddBackslashW are unnecessary:  the W variant doesn't
have to deal with MBCS string encodings, so using strlenW to find the
end of the string is sufficient.  Besides, the following change is
simply incorrect, and will produce incorrect results nearly all the
time:
+  if(MAX_PATH <= lstrlen(lpszPath))

Thanks for contributing to Wine.
--Juan