Re: GNOME CVS: NetworkManager dcbw

[Christopher Aillon <caillon redhat com>]

On 10/24/2005 7:03 PM, Dan Williams wrote:

> 	- Track animation timeout using a gboolean rather than the timeout's
> 		ID, since timeout IDs can legally be 0.



Um, no they can't.... See gmain.c[1]:  A GMainContext has a next_id member which is initialized to 1.

GMainContext *
g_main_context_new (void)
{
 GMainContext *context = g_new0 (GMainContext, 1);

 /* ... */

 context->ref_count = 1;

 context->next_id = 1;

 /* ... */
}


g_timeout_add () calls g_timeout_add_full which does:

guint
g_timeout_add_full (gint           priority,
		    guint          interval,
		    GSourceFunc    function,
		    gpointer       data,
		    GDestroyNotify notify)
{
 GSource *source;
 guint id;
 
 /* ... */

 id = g_source_attach (source, NULL);
 g_source_unref (source);

 return id;
}

and looking at g_source_attach ()

guint
g_source_attach (GSource      *source,
		 GMainContext *context)
{
 guint result = 0;
 GSList *tmp_list;

 g_return_val_if_fail (source->context == NULL, 0);
 g_return_val_if_fail (!SOURCE_DESTROYED (source), 0);

 /* ... */

 result = source->source_id = context->next_id++;

 /* ... */

 return result;
}


If we are getting 0 returned from g_timeout_add() it is a failure case we should be handling, and not assuming that everything is OK as we do now incorrectly with your patch.  The previous code was correct, so I suggest reverting this change.  Was there a specific issue you were trying to solve?


[1] http://cvs.gnome.org/viewcvs/glib/glib/gmain.c?rev=1.131&view=markup