Re: gdk_rgb_init calls for robustness

[Darin Adler <darin eazel com>]

on 1/23/01 3:07 PM, Havoc Pennington at hp redhat com wrote:

> Darin Adler <darin eazel com> writes:
>> If we want to go the other way and reaffirm that we require explicit
>> gdk_rgb_init calls, an alternative would be to put in return_if_fail calls
>> that check if image_info is NULL in these same places, to help people notice
>> when they forget to call gdk_rgb_init.
> 
> I would consider that correct - we do require gdk_rgb_init() calls for
> 1.2.x where x <= 8, so if we fix it in x = 9 people can accidentally
> ship apps that require 1.2.9, which is not really encouraged.

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.

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.

Index: gdk/gdkrgb.c
===================================================================
RCS file: /cvs/gnome/gtk+/gdk/gdkrgb.c,v
retrieving revision 1.24.2.3
diff -p -u -r1.24.2.3 gdkrgb.c
--- gdk/gdkrgb.c    2000/11/18 16:59:40    1.24.2.3
+++ gdk/gdkrgb.c    2001/01/23 23:34:25
@@ -726,6 +726,8 @@ gdk_rgb_xpixel_from_rgb (guint32 rgb)
 {
   gulong pixel = 0;
 
+  g_return_val_if_fail (image_info != NULL, 0);
+
   if (image_info->bitmap)
     {
       return ((rgb & 0xff0000) >> 16) +
@@ -2923,7 +2925,7 @@ static gint sincelast;
 #undef NO_FLUSH
 
 static gint
-gdk_rgb_alloc_scratch_image ()
+gdk_rgb_alloc_scratch_image (void)
 {
   if (static_image_idx == N_REGIONS)
     {
@@ -3090,6 +3092,7 @@ gdk_draw_rgb_image (GdkDrawable *drawabl
             guchar *rgb_buf,
             gint rowstride)
 {
+  g_return_if_fail (image_info != NULL);
   if (dith == GDK_RGB_DITHER_NONE || (dith == GDK_RGB_DITHER_NORMAL &&
                       !image_info->dith_default))
     gdk_draw_rgb_image_core (drawable, gc, x, y, width, height,
@@ -3114,6 +3117,7 @@ gdk_draw_rgb_image_dithalign (GdkDrawabl
                   gint xdith,
                   gint ydith)
 {
+  g_return_if_fail (image_info != NULL);
   if (dith == GDK_RGB_DITHER_NONE || (dith == GDK_RGB_DITHER_NORMAL &&
                       !image_info->dith_default))
     gdk_draw_rgb_image_core (drawable, gc, x, y, width, height,
@@ -3136,6 +3140,7 @@ gdk_draw_rgb_32_image (GdkDrawable *draw
                guchar *buf,
                gint rowstride)
 {
+  g_return_if_fail (image_info != NULL);
   if (dith == GDK_RGB_DITHER_NONE || (dith == GDK_RGB_DITHER_NORMAL &&
                       !image_info->dith_default))
     gdk_draw_rgb_image_core (drawable, gc, x, y, width, height,
@@ -3169,6 +3174,8 @@ gdk_draw_gray_image (GdkDrawable *drawab
              guchar *buf,
              gint rowstride)
 {
+  g_return_if_fail (image_info != NULL);
+
   if (image_info->bpp == 1 &&
       image_info->gray_cmap == NULL &&
       (image_info->visual->type == GDK_VISUAL_PSEUDO_COLOR ||
@@ -3195,6 +3202,8 @@ gdk_rgb_cmap_new (guint32 *colors, gint
 
   g_return_val_if_fail (n_colors >= 0, NULL);
   g_return_val_if_fail (n_colors <= 256, NULL);
+  g_return_val_if_fail (image_info != NULL, NULL);
+
   cmap = g_new (GdkRgbCmap, 1);
   memcpy (cmap->colors, colors, n_colors * sizeof(guint32));
   if (image_info->bpp == 1 &&
@@ -3232,6 +3241,7 @@ gdk_draw_indexed_image (GdkDrawable *dra
             gint rowstride,
             GdkRgbCmap *cmap)
 {
+  g_return_if_fail (image_info != NULL);
   if (dith == GDK_RGB_DITHER_NONE || (dith == GDK_RGB_DITHER_NORMAL &&
                       !image_info->dith_default))
     gdk_draw_rgb_image_core (drawable, gc, x, y, width, height,
@@ -3246,6 +3256,7 @@ gdk_draw_indexed_image (GdkDrawable *dra
 gboolean
 gdk_rgb_ditherable (void)
 {
+  g_return_val_if_fail (image_info != NULL, FALSE);
   return (image_info->conv != image_info->conv_d);
 }
===================================================================

    -- Darin