[virt-tools-list] [virt-viewer PATCH] Test error->message and, if NULL, use a default message

Jonathon Jongsma jjongsma at redhat.com
Fri Feb 10 20:14:30 UTC 2017


On Fri, 2017-02-10 at 19:18 +0100, Christophe de Dinechin wrote:
> > On 10 Feb 2017, at 15:50, Pavel Grunt <pgrunt at redhat.com> wrote:
> > 
> > On Thu, 2017-02-09 at 18:46 +0100, Christophe de Dinechin wrote:
> > > I tend to agree. I don’t see how any legitimate GError * (from
> > > g_error_new or the like) would have a NULL message. So this is
> > > really defensive coding.
> > > 
> > 
> > nothing prevents calling g_error_new with a NULL message :) I know
> > it
> > is not our case
> 
> Actually, digging deeper, I think this is not valid. See https://gith
> ub.com/GNOME/glib/blob/master/glib/gerror.c#L406. There is a
> g_warn_if_fail in that case. Also, the error message is the result of
> a strdup, and is allocated. And at
> https://github.com/GNOME/glib/blob/master/glib/gerror.c#L491, the
> call to g_free is unconditional. So it looks to me like the internal
> assumption in all these routines is that message cannot be NULL.
> 
> That being said, nothing prevents you from doing something like
> 
> 	GError *bad_error = malloc(27);
> 
> In that case, we are pretty much screwed.
> 
> So in light of the g_warn_if_fail, I think that my second patch
> should be rejected.
> 

I agree, I think we can drop that second patch. If it were necessary,
we would have a lot of code to fix in other projects as well ;)

Jonathon




More information about the virt-tools-list mailing list