[UPDATE] [programs/uninstaller] Check HKCU for uninstall
entries too
Tom Spear
speeddymon at gmail.com
Mon Apr 23 17:42:52 CDT 2007
Well, I finally solved all of my issues with my patch (yay!). It will
be ready to submit to wine-patches as soon as I reinstall wine to
finish up my testing.. Something borked it and now it won't run even
winecfg... Go figure. So anyways, with any luck, my testing will go
without a hitch, and I can get my first patch ever done in C for the
wine project committed. Wish me luck.
Tom
On 4/19/07, Tom Spear <speeddymon at gmail.com> wrote:
> 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
>
--
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
More information about the wine-devel
mailing list