Re: gdk_rgb_init calls for robustness



Darin Adler <darin eazel com> writes: 
> I see. I guess I was thrown off by the gdk_rgb_getcmap and
> gdk_rgb_get_visual calls, which both do call gdk_rgb_init. The inconsistency
> makes it very easy to do a lot of testing without realizing you are missing
> the needed gdk_rgb_init calls.
> 
> I guess then that my (alternative) proposed change isn't so useful then. It
> just changes seg. faults into return_if_fail calls, which are likely to be
> quickly followed by other failures anyway.
> 
> I'm showing the proposed patch to add the return_if_fail anyway, in case
> someone wants to encourage me to do it despite my misgivings.
>

That patch looks good. The only change I'd make is to use an
explanatory warning instead of g_return_if_fail (because 
"assertion failed: image_info != NULL" won't mean much to most
people).

i.e. 
static void
gdk_rgb_check_init (void)
{
  if (image_info == NULL) 
    g_warning ("gdk_rgb_init() must be called before using gdk_rgb_ functions");
}
 
Then s/g_return_if_fail (image_info != NULL)/gdk_rgb_check_init()/g;

If you want to commit that fix to the stable branch, please go ahead
(I cleared it with Owen).

> One further alternative is to do a g_warning, but then go ahead and do the
> gdk_rgb_init, in 1.2.9. This would let the developer know that something
> needs fixing, but not penalize the hapless user who happens to exercise the
> code path that doesn't get the gdk_rgb_init call.

As a policy, we don't try to recover from these assertion failures, it
just clutters up the code and makes it harder to maintain.

Of course in this case, the correct policy was to simply not have an
_init() function when we could have saved the developer that headache,
and in GTK 2.0 that's what's been done. Also I recently stripped
gdk_pixbuf_init() from HEAD, and we owe Tim a mail giving a rationale
for removing g_type_init() which I think is a good idea.

Basically all _init() functions that don't take arguments are pretty
gratuitous, they can always be called automatically.

Thanks!

Havoc






[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]