Re: warning about invalid tree model iterators

On Tue, 21 Mar 2006, Yevgen Muntyan wrote:

Tim Janik wrote:

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

thanks for pointing this out.
first, i have to admit that i based my initial decision on the fact that adding
G_GNUC_WARN_UNUSED_RESULT to the tree API generates *0* warnings with CVS gtk+.

and - that is indeed the case if you compile with gcc-3.3 ;)

gcc-3.3 simply has no support for the flag, and i erroneously used it to
benchmark the effect of flagging the tree view API. compiling with gcc-3.4
does show the loads of warnings you describe.

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

valid point. i was willing to accept a *few* bogus warnings, but introducing
a major inconvenience in the tree view API is not useful.

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);).

disabling G_GNUC_WARN_UNUSED_RESULT globally is not what anyone of us had
in mind i think. i agree with you that the patch should simply be backed
out where it generates too many warnings.

did you, or anyone else for that matter, investigate yet whether it'd be worth
keeping the warnings for *some* of the functions?

Best regards,


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