Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
To: Austin Clements <amdragon@MIT.EDU>, 
From: Justus Winter <4winter@informatik.uni-hamburg.de>
In-Reply-To: <20120922161908.GF26662@mit.edu>
References:
 <1348231837-21700-1-git-send-email-4winter@informatik.uni-hamburg.de>
	<1348231837-21700-2-git-send-email-4winter@informatik.uni-hamburg.de>
	<20120922161908.GF26662@mit.edu>
Message-ID: <20120924125510.13910.18774@thinkbox.jade-hamburg.de>
User-Agent: alot/0.3.3+
Subject: Re: [PATCH 2/2] Avoid potentially dereferencing a NULL pointer
Date: Mon, 24 Sep 2012 14:55:10 +0200
Cc: notmuch@notmuchmail.org
Precedence: list

Quoting Austin Clements (2012-09-22 18:19:08)
> Quoth Justus Winter on Sep 21 at  2:50 pm:
> > GMIME_IS_MULTIPART and GMIME_IS_MESSAGE both handle NULL pointers
> > gracefully, but the G_OBJECT_TYPE used in the error handling block
> > dereferences it without checking it first.
> > =

> > Fix this by checking whether parent->part is valid.
> > =

> > Found using the clang static analyzer.
> =

> Neat.

Yes. Besides this the code turns up no warnings (modulo one false
positive, clang doesn't understand that progress_notify is never
called if it's NULL in notmuch_database_upgrade b/c the signal handler
isn't set up then).

> Can this actually happen, though?  If so, I think this point is too
> late to be checking for a NULL part field.  It should probably be
> checked when the mime_node_t is created so that mime_node_t never has
> a NULL part field.

I'm not sure actually. Then again this patch isn't hacky at all and
being somewhat defensive isn't bad imho.

Justus