Re: warning about invalid tree model iterators

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

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

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,

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