[EMAIL PROTECTED] writes:
> > If width is unsigned and 10, then:
> >
> > int new_width = MAX (width - 15, 1);
> >
> > produces 4294967291, not 1.
>
> This is clearly a bug in the code. There are sizes which are by
> definition unsigned and code computes an negative value this is
> about as harmful as an big positive one.
This is not a bug in the code, it's only a bug if the code uses
unsigned. Why on earth should we introduce side conditions that make
using perfectly ok code like above buggy? Using signed is just
defensive programming, using unsigned introduces side effects that are
hard to find.
> > Also, in lots of places you've changed, the value -1 was used to
> > indicate an unset value (width and height of GimpPreview for
> > example).
>
> Shouldn't be. I was about to go even further and change all false uses
> of such values. What you imply is that using unsigned types for unsigned
> values is wrong because you also use them for errorindication and this
> (together with magic constants) is about most of the evilness of
> programming.
I don't see why the type "preview size" which is defined as
"size of the preview if => 0, or undefined otherwise" is bad.
What special value would you introduce instead?
> > This happens when values others than TRUE (1) and FALSE (0) are
> > assigned to struct fields defined as unsigned int : 1.
>
> What do you expect to happen then?
The following will happen:
struct eek
{
guint argh : 1;
};
void
foobar (struct eek eek,
gboolean baz)
{
eek.argh = baz;
}
Now calling foobar(my_eek, 2);
will result in my_eek.argh being 0 (the least significant bit of "2").
Assigning to a gboolean (which is a *signed* int) will assign 2
and still be TRUE in the C sense.
> > you changed code from
> > gimp->be_verbose = be_verbose ? TRUE : FALSE;
> > to
> > gimp->be_verbose = be_verbose;
>
> > This is wrong, especially if a bitfield is used. I don't think I need
> > to explain the reason.
>
> It doesn't even make sense when NOT using bitfields but I definitely see
> no problem when doing so. Please don't always assume I see what you mean
> by telling "this is wrong"; if you see a problem here tell me so.
It makes sense and kindof *must* be used for unsigned bitfields. See above.
Furthermore, I don't see a reason for being pissed of. If you announced
what you were about to do we could have discussed it beforehand.
ciao,
--Mitch
_______________________________________________
Gimp-developer mailing list
[EMAIL PROTECTED]
http://lists.xcf.berkeley.edu/mailman/listinfo/gimp-developer
