dlls/msi/string.c -- fix use of signed versus unsigned (RESEND)

James Hawkins truiken at gmail.com
Thu Nov 15 11:08:01 CST 2007


On 11/15/07, Gerald Pfeifer <gerald at pfeifer.com> wrote:
> On Thu, 15 Nov 2007, James Hawkins wrote:
> >> ChangeLog:
> >> Fix use of signed versus unsigned variables.
> > This change is unnecessarily complicated.
>
> How do you propose to address the following?
>
>   string.c: In function 'msi_addstring':
>   string.c:208: warning: comparison between signed and unsigned
>   string.c: In function 'msi_addstringW':
>   string.c:260: warning: comparison between signed and unsigned
>   string.c: In function 'msi_string2idW':
>   string.c:400: warning: comparison between signed and unsigned
>

Turn off the warning.

> > Also, the only actual bug of this type that was in string.c
> > (comparison..always false) has been fixed:
>
> Happy to hear that!
>
> > All these patches to silence extraneous warnings makes me wonder whether
> > you really understand the code or if you're just trying to get rid of
> > warnings...
>
> I'm mostly trying to get bugs fixed.  In the last couple of weeks my
> patches have addressed several realy bugs and/or triggered others to
> look into bugs, both of which I'd claim is a good thing. ;-)
>

Ok, but this patch didn't fix any bug and it's makes an already
complicated module that much more complicated.

> Reducing the number of warnings is one way to catch regressions easier
> and get bugs addressed (not all warnings really are due to real bugs,
> of course, but that's hard to see up front).
>

I don't see how silencing warnings catches any regression.  It's hard
to see for you because you're changing code you don't understand.

> And, no, I am not an expert of the entire Wine codebase, and certainly
> not to the level you and others are for those pieces you maintain.  And
> the MSI code apparently is one of the more tricky ones...
>

We all appreciate your effort, but we could use your resources in more
helpful ways.  I suggest you look through bugzilla, pick out a bug,
and try to fix that bug.  You'll become more familiar with the code
that way and you'll be fixing an actual reported bug.  If you really
want to keep sending in patches for these warnings, please be more
conservative.  Unsigned comparisons for less than 0 are actual bugs,
so you can be sure about those, but the other warnings you need to
spend a lot of time to see if there really is any bug.  Warnings are
just warnings, not errors, and there's a reason we don't turn all the
warnings on.

-- 
James Hawkins



More information about the wine-devel mailing list