Re: [UPDATE] [programs/uninstaller] Check HKCU for uninstall entries too

[Tom Spear]

Try applying the patch before you comment on it.  See below..

On 4/19/07, James Hawkins  wrote:
> On 4/19/07, Tom Spear  wrote:
> > Apparently the last patch had multiple patches in the file because I
> > didnt delete it before creating the new one..
> >
> > So here we go again.
> >
> > Check HKCU as well as HKLM for uninstall entries, using a for loop
>
> @@ -69,7 +69,7 @@
>  /**
> - Used to output program list when used with --list
>
> */
> -static void ListUninstallPrograms(void)
> +static void ListUninstallPrograms()
>
> The parameter list was correct as is.  If a function takes no
> parameters, the correct list is 'void'.  Don't change parts of the
> code that don't have to do with your patch, especially when you don't
> know what you're changing.

That one was an oops, I'll change it back...
>  if (i < numentries)
> -
>    UninstallProgram();
> -
>    for (x=0; x<2; x++)
> -
>        UninstallProgram(rootkey);
>
> Why are you calling UninstallProgram twice with the same key?  Besides
> that, using magic numbers (2) is bad coding.

That is another oops.  I should have double checked this as well.  The
use of the 'magic number' in this case is that there will never be
more than 2 keys (EVER) to check.  However I will make a constant
array instead and check x against the number of values in this array
instead.
> - HKEY keys[2];
> - keys[0] = HKEY_CURRENT_USER;
> - keys[1] = HKEY_LOCAL_MACHINE;
>
> HKEY keys[] = { HKEY_CURRENT_USER, HKEY_LOCAL_MACHINE };
> -
>   for (x=0; x<2; x++)
>
> You're hardcoding this value to 2, which is another bad magic number.
> What if, hypothetically, more root keys are added to the list?

see above
> for (x = 0; x < sizeof(keys); x++)
> - rootkey[0] = HKEY_CURRENT_USER;
> - rootkey[1] = HKEY_LOCAL_MACHINE;
>
> Same thing as above.

see above
> -
>    sizeOfSubKeyName = 255;
>
> What is 255?

I dont know, but if you look at the file (pre patch) that is already
there, diff just deletes and readds it because it moved further down
in the file.  However, to answer your question, I checked the MS
website:

http://support.microsoft.com/kb/256986

<quote>
The maximum size of a key name is 255 characters.

The following table lists the data types that are currently defined
and that are used by Windows. The maximum size of a value name is as
follows:
•	Windows Server 2003 and Windows XP: 16,383 characters
•	Windows 2000: 260 ANSI characters or 16,383 Unicode characters
•	Windows Millennium Edition/Windows 98/Windows 95: 255 characters
</quote>
> - HKEY rootkey[2];
> - rootkey[0] = HKEY_CURRENT_USER;
> - rootkey[1] = HKEY_LOCAL_MACHINE;
>
> Same.

same
> -
>                    /* FIXME: UninstallProgram should figure out
>
> which root key to uninstall
> -
>                     * from, as opposed to just going with the
>
> first value in rootkey.  The
> -
>                     * entry gets removed this way as well, but
>
> it is not apparent by this code.
> -
>                     */
> -
>                    UninstallProgram(*rootkey);
>
> What is the point of adding rootkey if you're not going to use it?
> UninstallProgram(HKEY_X) is a lot shorter.  The point is, don't add
> something until you're going to use it.  On top of that, this new code
> is basically UninstallProgram(HKEY_CURRENT_USER) when the original
> code was (parameter added for convenience)
> UninstallProgram(HKEY_LOCAL_MACHINE).  That's not right.

Well, unfortunately there isnt a way to call UninstallProgram without
a parameter if it is declared that way.  So I could either hardcode it
to HKEY_LOCAL_MACHINE, since it requires a parameter now, or I could
leave it as *rootkey to be fixed later.  I tried to find a way to for
loop it, but because it is inside DlgProc, I get an error when
compiling.  So I left it alone..  As the comment says, the code works
as is, but it is not apparent by the code.  If someone has a
suggestion on how to get that specific line of code to look proper (so
you can tell if it is doing UninstallProgram on HKLM or HKCU, please
feel free..

The reason we need the parameter added to UninstallProgram is because of:

/* delete the application's uninstall entry */
RegOpenKeyExW(HKEY_LOCAL_MACHINE, PathUninstallW, 0, KEY_READ, &hkeyUninst);

being in the original file.  I think it's better to have whichever
root key is being used passed to the RegOpenKeyExW.....

To be honest I was more worried about someone not liking my use of the
for loop in FetchUninstallInformation...
> --
> James Hawkins