Request for Patchreview

Juan Lang juan.lang at gmail.com
Mon Jun 15 13:06:02 CDT 2009


Thanks André.

> My tests worked with Instal*.exe files on english Windows, too.
> I bet it is because of the instal~1.exe writing. (8.3 and 6##.3)

Ah, right.  Very good, leave this as it is then.

Sorry for the sloppy review, there are a couple more things I'd like
to ask about:

> My tests worked with Instal*.exe files on english Windows, too.
> I bet it is because of the instal~1.exe writing. (8.3 and 6##.3)
> So i correct some things:
> http://www.winehq.org/pipermail/wine-patches/2009-June/074266.html

+static WCHAR FileNameBufferW[MAX_PATH];
+static WCHAR FileTitleBufferW[MAX_PATH];
+static const WCHAR openW[] = {'o','p','e','n',0};

Why are these static?  openW is probably fine as a static, as it's
const, but the other two are never used beyond the InstallProgram
scope, and therefore shouldn't be static.  As a general rule, you
should make variables' scope as narrow as possible.

One minor enhancement along these lines:
+    SHELLEXECUTEINFOW sei;

this isn't used in the outer block, only in the if
(GetOpenFileNameW(&ofn)) block.  Therefore it shouldn't be declared
until the block in which it's used.

Style:
+    if (GetOpenFileNameW(&ofn)) {
the existing code puts braces on a newline.  Please do the same.

Finally,
+    return;
an empty return statement is superfluous.  Please omit it.
--Juan



More information about the wine-devel mailing list