DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma;
	h=mime-version:sender:in-reply-to:references:date
	:x-google-sender-auth:message-id:subject:from:to:cc:content-type
	:content-transfer-encoding;
	b=cWrH6GQNIqcF1OuqlliWtkaTOYti1pwcB532p1bY7H5wpGUkpq04ZT/erCITvBnEuM
	xi/ss0/UZzMsEHXDOQjHHDM6a9vd01LjpkNryr9NxUci1YG5ylkJqDfOaI5iK6d+P8Yn
	v7nqKhdyCxOMxH+HTJOofwBQDNL2KGroUxDh4=
MIME-Version: 1.0
Sender: anthony.j.towns@gmail.com
In-Reply-To: <877holavq5.fsf@ut.hh.sledj.net>
References: <877holavq5.fsf@ut.hh.sledj.net>
Date: Tue, 6 Apr 2010 18:17:44 +1000
Message-ID: <h2y87b3a4191004060117v5421db8ejbe3030d0626e7440@mail.gmail.com>
Subject: Re: [PATCH] json: Avoid calling strlen(NULL)
From: Anthony Towns <aj@erisian.com.au>
To: David Edmondson <dme@dme.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Cc: notmuch <notmuch@notmuchmail.org>
Precedence: list

On Tue, Apr 6, 2010 at 17:25, David Edmondson <dme@dme.org> wrote:
> =A0 =A0json: Avoid calling strlen(NULL)
> =A0 =A0MIME parts may have no filename, which previously resulted in call=
ing
> =A0 =A0strlen(NULL).
> =A0char *
> =A0json_quote_str(const void *ctx, const char *str)
> =A0{
> + =A0 =A0if (str =3D=3D NULL)
> + =A0 =A0 =A0 return (char *)"\"\"";
> +
> =A0 =A0 return (json_quote_chararray (ctx, str, strlen (str)));
> =A0}

There's already a check in json_quote_chararray for len=3D=3D0, so it
might be sensible to say:

    return (json_quote_chararray (ctx, str, str !=3D NULL ? strlen (str) : =
0));

OTOH, the code in json_quote_array to deal with that does the same
thing (returns a literal string containing two quote marks), which
seems wrong -- the normal code path is to talloc to get a newly
allocated, editable string, that might be talloc_free'd later,
wouldn't it make more sense just to let the str=3D=3DNULL / len=3D=3D0
behaviour fall through into the normal case code?

FWIW:

commit 5b93a488221b50c02db18d86a550cb3c038c00da
Author: Anthony <aj@erisian.com.au>
Date:   Tue Apr 6 18:10:39 2010 +1000

    json: Avoid calling strlen(NULL), and always return a newly talloced ar=
ray.

    MIME parts may have a no filename, which causes json_quote_str()
to be invoked
    with NULL instead of a string.

diff --git a/json.c b/json.c
index f90b0fa..5e379ef 100644
--- a/json.c
+++ b/json.c
@@ -57,9 +57,6 @@ json_quote_chararray(const void *ctx, const char
*str, const size_t len)
     size_t loop;
     size_t required;

-    if (len =3D=3D 0)
-	return (char *)"\"\"";
-
     for (loop =3D 0, required =3D 0, ptr =3D str;
 	 loop < len;
 	 loop++, required++, ptr++) {
@@ -105,5 +102,8 @@ json_quote_chararray(const void *ctx, const char
*str, const size_t len)
 char *
 json_quote_str(const void *ctx, const char *str)
 {
+    if (str =3D=3D NULL)
+        str =3D "";
+
     return (json_quote_chararray (ctx, str, strlen (str)));
 }



--=20
Anthony Towns <aj@erisian.com.au>