Re: warning about invalid tree model iterators

[Yevgen Muntyan <muntyan tamu edu>]

Tim Janik wrote:

> tree model iterators are easily advanced/setup/created in a way that 
> yields
> an invalid iterator, because the denoted location doesn't exist 
> (anymore).
> to cath those cases, the tree model API returns booleans indicating 
> success,
> but testing those booleans is easily forgotton and a common error source.
>
> to help the compiler catch these mistakes, i've prepared a patch that 
> adds
> G_GNUC_WARN_UNUSED_RESULT to all relevant iterator functions, and intend
> to commit that next week unless objections pop up. in principle it does:

These warnings flags generate too many false warnings, i.e. warnings in 
cases
where it's perfectly okay to ignore return value.
Examples of such uses are the following:
1) When you know exactly what your model contains, e.g. when you know 
it's not
empty, and you are getting iter pointing to the first node. Or, when you 
get number
of nodes in the model, and then iterating over them in a for loop.
2) When you get path/iterator somehow, with error checking, and then 
convert it
to iterator/path. E.g. you can get a path from a row reference, and if 
it's not NULL,
you know it's a valid path, so you don't need to check return value of
gtk_tree_model_get_iter.

These two cases pretty much cover all the methods with added warnings 
(watch
gtk compilation for number of examples of wrong warnings), so while 
these warnings
may help to catch bugs, they also help to add new bugs, since one either 
starts
ignoring warnings or is forced to suppress warnings using dummy
variables.

Therefore, either these warnings flags should be removed, or there 
should be
introduced something that allows one to disable them (or rather enable).
I believe the real solution would be using some tool like splint which would
allow to suppress/force warnings in given places. GCC attribute doesn't
seem to be intelligent enough to handle GtkTreeModel api ;)

Note that disabling G_GNUC_WARN_UNUSED_RESULT globally would not be
a nice solution, since it's usually good to have unused-result warnings from
GList api, for example; and it's much less common to ignore return value
of, say, g_list_delete_link (though, of course, it's possible to write 
code like
list = g_list_append(NULL, NULL); g_list_delete_link(list, list);).

Best regards,
Yevgen