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

[Tom Spear]

I rewrote the patch based on your suggestions, and I'm having a little
difficulty with certain things.  I did a little more testing, and it
turns out that the uninstaller I was using removes the local machine
uninstall entry on its own.  So I started futzing with the code some
to try to figure a way to get the root key to be properly passed to
UninstallProgram.  The only thing I can come up with is some form of
using entries[sel].key.  I can only test at work though, so I have no
way of knowing until tomorrow if that would even work.  However,
assuming that it will, is there a way to strip PathUninstallW from
entries[sel].key so that the only thing left is the root key?  If so
then I can probably use something along the lines of (pseudocode):

root_key = strip(entries[sel].key, PathUninstallW);
UninstallProgram(root_key);

Thanks for the help

Tom

On 4/19/07, James Hawkins <truiken at gmail.com> wrote:
> On 4/19/07, Tom Spear <speeddymon at gmail.com> wrote:
> > Try applying the patch before you comment on it.  See below..
> >
>
> I read all your reply comments, and nothing you said leads me to
> believe that I need to apply this patch to review it.
>
> > On 4/19/07, James Hawkins <truiken at gmail.com> wrote:
> > > On 4/19/07, Tom Spear <speeddymon at gmail.com> 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.
> >
>
> Engineers at a certain software company decided that machines would
> not have more than 640k of RAM anytime soon, leading to at least a
> decade of headaches for developers.  Case in point, putting
> self-imposed limits in your code is a bad design habit.
>
> > > +    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:
> >
>
> The old code is clearly in the patch, and no one implied that you
> wrote it.  The comment still stands though.  The 255 needs to be
> replaced with a const or define that clarifies what the value
> represents.
>
> > 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.
>
> Please reread my comment.  rootkey is superfluous, not the parameter
> to UninstallProgram.
>
> > 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.
>
> Since rootkey shouldn't be in the patch (should only be added when
> it's used), the only option remaining is to call
> UninstallProgram(HKEY_LOCAL_MACHINE).
>
> >  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.....
> >
>
> I never had any objections with the new parameter to UninstallProgram.
>
> --
> James Hawkins
>


-- 
Thanks

Tom

Check out this new 3D Instant Messenger called IMVU.  It's the best I
have seen yet!



http://imvu.com/catalog/web_invitation.php?userId=1547373&from=power-email