Web lists-archives.com

Re: [PATCH] Fix check for 'y' padding in gtk_cell_renderer_set_padding()




On Tue, 2015-01-13 at 08:10 -0500, Matthias Clasen wrote:
> On Mon, Jan 12, 2015 at 1:30 PM, Philip Withnall <philip@xxxxxxxxxxxxxxx> wrote:
> > On Sun, 2015-01-11 at 16:43 +0000, Emmanuele Bassi wrote:
> >> hi;
> >>
> >> thanks for your patches. GTK+ does not use the mailing list to track
> >> bugs and patches; could you please open bugs on Bugzilla, instead?
> >>
> >> just use: https://bugzilla.gnome.org/enter_bug.cgi?product=gtk%2B
> >>
> >> this will make it easier for us to track the issue, review the code,
> >> and commit the change.
> >
> > This looks similar to (but not the same as) the patches I posted from
> > analysing GTK+ with Clang static analyser in November, which are still
> > awaiting review.
> >
> > I really hope we aren't going to end up with duplicate bug reports and
> > duplicate patches because things are not getting reviewed. :'-(
> >
> > https://bugzilla.gnome.org/show_bug.cgi?id=712760
> >
> 
> I can't say that I agree with them being very similar. Most of Maks'
> patches were small, to the point, obvious oversight fixes that could
> be reviewed in a minute, each. The static analysis 'fixes' are an
> endless stream of mind-numbing can-never-happen corner cases of
> dubious value. Just IMO, of course :-)

He’s doing great work, for sure.

Initially I posted a lot of static analysis ‘fixes’, purely because I
didn’t know how (entirely reasonably) picky you wanted to be about what
to consider relevant. There has not been an endless stream of them: I’ve
stalled on this Coverity work (on GTK+ at least) since the end of 2013.

Since the first round of patches, I have tried to be more selective
about which issues I patch, and have marked a lot more as false
positives in Coverity. I believe the patches currently on bug #712770
(and a couple of GLib bugs) are all now mostly relevant bugs which can
be triggered in the real world.

For example, this one’s quite similar to one of the problems Maks fixed:
https://bugzilla.gnome.org/show_bug.cgi?id=712760#c103

This one is triggerable (IIRC) by calling gtk_widget_set_visual(widget,
NULL):
https://bugzilla.gnome.org/show_bug.cgi?id=712760#c106

This one fixes handling NULL page setup objects when printing:
https://bugzilla.gnome.org/show_bug.cgi?id=712760#c107

If you still think the balance is wrong, let me know, and we can try and
sort out some rules for what kinds of issues to consider patching.

I believe that fixing static analysis errors (or marking them as
ignored) is like fixing compiler warnings: a lot of fiddling around in
the beginning to get the code clean, and then the value comes from
seeing new errors come in with new changes.

Please, either review the patches; or reject them all with a big notice
somewhere saying that GLib is not expected to be Coverity-clean and I’ll
stop wasting my time trying to make static analysis useful. Either
outcome would be fine. Just don’t leave the patches unreviewed
indefinitely. That’s the worst of all worlds.

Philip

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

_______________________________________________
gtk-devel-list mailing list
gtk-devel-list@xxxxxxxxx
https://mail.gnome.org/mailman/listinfo/gtk-devel-list