Re: RFC: warnings on ignoring return value on some list operations



On Thu, 2005-11-24 at 17:19 +0000, Gustavo J. A. M. Carneiro wrote:
> Qui, 2005-11-24 às 16:35 +0100, Tim Janik escreveu:
> > On Thu, 24 Nov 2005, Owen Taylor wrote:
> > 
> > > if (head) {
> > >    g_list_append(tail, new);
> > >    tail = tail->next;
> > > } else {
> > >    head = tail = g_list_append(null, new);
> > > }
> > >
> > > strikes me as acceptable code, and I know there are some examples like this
> > > in GTK+. Maybe the gain is worth the pain ... unless someone is compiling
> > > production code with -Werror it isn't going to *break* a build, and there
> > > is no bin-compat issue. But it's definitely a compatibility break of some
> > > sort.
> > 
> > i think one can argue both ways here, in the above code, i'd still write
> > head = g_list_append (tail, new); and recommend that people also do that,
> > because a) code is duplicated so often and this easily introduces errors
> > in another context, and b) i'd like to think of the list API as something
> > where i'm not allowed to ignore return values to avoid mistakes ;)
> > 
> > >
> > > Does
> > >
> > >   (void)g_list_append(tail, new);
> > >
> > > Suppress the warning?
> > 
> > i was assuming that, unfortunately that is not the case:
> 
>   I don't see how:
> 
> 	(void)g_list_append(tail, new);
> 
> is any better than:
> 
> 	tail = g_list_append(tail, new);

I think it's clearer - if tail actually could chang, then the code I
exhibited would be broken, since tail->prev / tail->prev->next
would have been left pointing to the wrong place.

> and that still leaves the option:
> 
> 	GList *dummy = g_list_append(tail, new);

This doesn't work - it will produce
"unused variable ‘dummy’. And it might sometime in the future start
producing the warning about an unchecked result ... it *is* clearly
unchecked to the compiler anyways.

I'm not really opposed to adding a warning to g_list_append() - after
all, there are no operations on GList where you provably *have* to 
check the result - unlike GSList.

But it does suck that that there is no clear and obvious way to 
suppress the warning. I'd be curious to see what the patch to fix up
GTK+ after adding the attributes looks like.

Regards,
						Owen

Attachment: signature.asc
Description: This is a digitally signed message part



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