Re: Converting source to unicode, bug 28189

[Akihiro Sagawa]

On Thu, 13 Feb 2014 07:35:21 -0500, Hugh McMaster wrote:
> I've put together a draft patch to address bug 28189, regsvr32:
> No usage in wineconsole.
>
> I'm using resource strings and have converted all of the printf calls
> to WriteConsoleW.
>
> I also converted the source to Unicode, but overall, there are few
> coding points I'm not quite sure about.
>
> If someone could look over the changes and offer some feedback,
> I'd certainly appreciate it.
>
> URL: http://bugs.winehq.org/show_bug.cgi?id=28189

Hello Hugh,

Regarding your patch, attachment #47519, I roughly read the code and
post comments at the following points:
1. Dead assignment: In output_write function, len and ret initial
   value will overwrite by function return value. You don't need
   to initialize them.
2. Memory leaking: if (!strA) is true, calling LocalFree(str) is
   necessary before return.
3. Usage indent: I'm not sure about STRING_USAGE and spaceSpaceW.
   Another command line tool, e.g. cmd.exe, doesn't do like this.
   Additionally, it might confuse po translators.
4. /I option handing: lstrcmpiW isn't compatible with strncasecmp.
5. Double quote handling: command_line is advanced when the code
   finds a double quote character. When it updates wsCommandLine?
6. Needless changes: `/**' -> `/*', pointer asterisk position, a
   space character after C-language keyword. Please follow surround
   coding style.
7. Silent handling: Many times, it refers Silent variable before
   calling output_write function. How about moving the check to the
   function?

Hope these comments are helpful.

Regards,
Akihiro Sagawa