On  3 Dec, Michael Natterer wrote:

> 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.

It is a bug in the code, if a value that is supposed to be positive
goes bezerk and it's just hidden by the fact that the value is in a
signed type then you'll be in trouble somewhere. Using unsigned values
is not only right but also demonstrates where the problems are because 
the compiler will do additional checks and if such a situation in a loop
occurs it'll be easy to track down because of the absurdity instead of
silently producing wrong results or leading to mysterious crashes later.
> 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?

Is a preview supposed to be of size 0? If yes we might want to take the
pointer of size return an gboolean, if no there is no problem at all.
But -1? Why not -2? 

>> 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.

Yes, yes. "Doctor, doctor, it hurts when I do this...". If this really
is an issue to you I can write you a script that checks that all given 
parameters of type gboolean are either TRUE or FALSE. I still hope that
at some point we'll globally have the type bool in all C compilers so
this won't be an issue at all.

> It makes sense and kindof *must* be used for unsigned bitfields. See
> above.

We could also introduce an assertion here if you're scared that our own
code (after all this is still within GIMP) will pass arbitrary values.

> 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.

I'm not at all pissed of that you spend a lot of time with backing me
out when it would have been a "patch -p0 -R <diff; cvs commit" to me.
I'm just annoyed by the fact that Sven adds me lots of burden because
he doesn't agree with my work.


Gimp-developer mailing list

Reply via email to