Date: Sat, 17 Mar 2012 13:26:32 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: Justus Winter <4winter@informatik.uni-hamburg.de>
Subject: Re: [PATCH] Fix error reporting in
	notmuch_database_find_message_by_filename
Message-ID: <20120317172632.GM2670@mit.edu>
References:
 <1332002487-32411-1-git-send-email-4winter@informatik.uni-hamburg.de>
	<1332002487-32411-2-git-send-email-4winter@informatik.uni-hamburg.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To:
 <1332002487-32411-2-git-send-email-4winter@informatik.uni-hamburg.de>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org
Precedence: list

Quoth Justus Winter on Mar 17 at  5:41 pm:
> Formerly it was possible for *message_ret to be left
> uninitialized. The documentation however clearly states that "[o]n any
> failure or when the message is not found, this function initializes
> '*message' to NULL".
> 
> Signed-off-by: Justus Winter <4winter@informatik.uni-hamburg.de>
> ---

Good catch.  Actually, it looks like the error handling path in this
function was completely bogus without this, since it checks if
*message_ret is non-NULL and destroys it if so, even though the
function may not have been the one to set it to NULL (and it may have
been uninitialized in the caller).

So, LGTM.

>  lib/database.cc |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/lib/database.cc b/lib/database.cc
> index 8f8df1a..16c4354 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -1825,6 +1825,9 @@ notmuch_database_find_message_by_filename (notmuch_database_t *notmuch,
>      if (message_ret == NULL)
>  	return NOTMUCH_STATUS_NULL_POINTER;
>  
> +    /* return NULL on any failure */
> +    *message_ret = NULL;
> +
>      local = talloc_new (notmuch);
>  
>      try {