New uninstaller

Dmitry Timoshkov dmitry at baikal.ru
Sun Feb 27 06:00:05 CST 2005


"Jonathan Ernst" <Jonathan at ErnstFamily.ch> wrote:

> I'm thinking to send it as is to wine-patches if nobody complains before
> tonight (tomorrow I'll be leaving for one week).

First of all thanks for your efforts. Since this is your first Win32 app
I personally understand all the difficulties you faced with. Please do not
be offenced by critical comments below, they are supposed to help you
better understand your mistakes and should serve as a guide for further
improvements.

> +int list_need_update = 1;
> +int oldsel = -1;
> +int FetchUninstallInformation(void);
> +char *sFilter;

I'd suggest to not mix variables and function declarations. Another
suggestion is to use static modifier for everything since nothing
is supposed to be externally visible.

If you could use unicode for everything internally that would be another
great bonus for us poor non latin1 using people. You are rewriting the app
almost entirely anyway. Just replace main by wmain and Wine startup code
will pass unicode strings to it, but make sure to use CP_UNIXCP to convert
strings from unicode to unix locale before printing them to the unix console.

Please use Win32 APIs for string comparisons since Wine and unix locales
are almost always not the same. Using unicode will drop this requirement.

Avoid using c++ comments, they are deprecated for Wine code.

Use consistent formatting through the code.

> +            if(sFilter != NULL && stristr(wine_dbgstr_w(entries[numentries-1].descr),sFilter)==NULL)
> +                numentries--;

wine_dbgstr_w is not supposed for the above use. Using unicode will help
to avoid such ugly constructs.

> +            RemoveSpecificProgram( argv[i++] );

argv[] list is passed in the unix locale while the code expects to see
it in the win32 locale. Using unicode will fix it as well.

> +char *stristr(const char *String, const char *Pattern)
> +{
> +    char *pptr, *sptr, *start;
> +    uint  slen, plen;
> +
> +    for (start = (char *)String,
> +         pptr  = (char *)Pattern,

Why not simply use "const char *" for pptr, sptr, start instead of silencing
compiler warnings and hiding potential bugs?

> +    {
> +        /* find start of pattern in string */
> +        while (toupper(*start) != toupper(*Pattern))

Again an issue with unix/win32 locale mixup.

-- 
Dmitry.





More information about the wine-devel mailing list