From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MA9P6-0007xb-RC for qemu-devel@nongnu.org; Fri, 29 May 2009 17:14:16 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MA9P1-0007vD-NF for qemu-devel@nongnu.org; Fri, 29 May 2009 17:14:16 -0400 Received: from [199.232.76.173] (port=39010 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MA9P1-0007v6-BC for qemu-devel@nongnu.org; Fri, 29 May 2009 17:14:11 -0400 Received: from smtp-out.google.com ([216.239.33.17]:53736) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1MA9Oz-0003Jj-UQ for qemu-devel@nongnu.org; Fri, 29 May 2009 17:14:11 -0400 Received: from wpaz1.hot.corp.google.com (wpaz1.hot.corp.google.com [172.24.198.65]) by smtp-out.google.com with ESMTP id n4TLCZO0002256 for ; Fri, 29 May 2009 22:12:35 +0100 Received: from fxm28 (fxm28.prod.google.com [10.184.13.28]) by wpaz1.hot.corp.google.com with ESMTP id n4TLCXp8018591 for ; Fri, 29 May 2009 14:12:33 -0700 Received: by fxm28 with SMTP id 28so6849148fxm.25 for ; Fri, 29 May 2009 14:12:32 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <200905291917.10535.jseward@acm.org> References: <200905290758.11551.jcd@tribudubois.net> <4A1FD6E2.9020006@redhat.com> <200905291407.26757.paul@codesourcery.com> <200905291917.10535.jseward@acm.org> Date: Fri, 29 May 2009 23:12:32 +0200 Message-ID: <60cad3f0905291412m670c7a6cw45f9b51f3122ddfb@mail.gmail.com> Subject: Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently From: David Turner Content-Type: multipart/alternative; boundary=001636c5a435ee9dfe046b138882 List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Julian Seward Cc: Kevin Wolf , Paul Brook , Gerd Hoffmann , qemu-devel@nongnu.org, Jean-Christophe Dubois --001636c5a435ee9dfe046b138882 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On Fri, May 29, 2009 at 7:17 PM, Julian Seward wrote: > > +1 for that. Code that relies on malloc(0) doing any specific thing > is basically bad news when it comes to portability, robustness > and understandability. Better to have qemu_malloc(0) abort, put up with > a couple of days of the trunk aborting, until these uses are fixed. > I'd be surprised if there were many cases anyway. > I think there are two conflicting goals here: - On one hand, people are suggesting to abort on malloc(0) because it is the sign of buggy code, and would help spot it as soon as possible, before any damage is done. - On the other hand, some people object that this assumption is sometimes wrong (e.g. with arrays), where having malloc(0) returning NULL or anything else is totally appropriate. Would it make sense to separate the two issues with something like the following: - qemu_malloc(0) aborts and print a panic message - qemu_calloc(count, itemsize) would return NULL for 'itemsize == 0', but would abort for 'count == 0' - array-allocating/parsing code MUST use qemu_calloc() exclusively. And calloc() can also perform trivial integer multiplication overflow checks too. I would even suggest providing helper macros to make the programmer's intent even more clear and less error-prone, as in: #define QEMU_NEW(ptr) (ptr) = qemu_alloc(sizeof(*(ptr))) #define QEMU_NEW_ARRAY(ptr,cnt) (ptr) = qemu_calloc((cnt),sizeof(*(ptr))) #define QEMU_RENEW_ARRAY(ptr,cnt) (ptr) = qemu_realloc((ptr),(cnt),sizeof(*(ptr))) #define QEMU_FREE_ARRAY(ptr) qemu_free(ptr) (yes, qemu_realloc() would take 3 parameters). Any direct use of malloc()/qemu_malloc() in source code would be suspicious and could easily spotted to check it. > > J > > > --001636c5a435ee9dfe046b138882 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

On Fri, May 29, 2009 at 7:17 PM, Julian = Seward <jseward@acm= .org> wrote:

+1 for that. =A0Code that relies on malloc(0) doing any specific thin= g
is basically bad news when it comes to portability, robustness
and understandability. =A0Better to have qemu_malloc(0) abort, put up with<= br> a couple of days of the trunk aborting, until these uses are fixed.
I'd be surprised if there were many cases anyway.

I think there are two = conflicting goals here:

- On one hand, people are suggesting to abor= t on malloc(0) because it is the sign of buggy code,
=A0 and would help = spot it as soon as possible, before any damage is done.

- On the other hand, some people object that this assumption is sometim= es wrong (e.g. with
=A0 arrays), where having malloc(0) returning NULL o= r anything else is totally appropriate.

Would it make sense to separ= ate the two issues with something like the following:

- qemu_malloc(0) aborts and print a panic message
- qemu_calloc(coun= t, itemsize) would return NULL for 'itemsize =3D=3D 0', but would a= bort for 'count =3D=3D 0'
- array-allocating/parsing code MUST u= se qemu_calloc() exclusively. And calloc() can also perform trivial
integer multiplication overflow checks too.

I would even suggest pro= viding helper macros to make the programmer's intent even more clearand less error-prone, as in:

#define=A0 QEMU_NEW(ptr)=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 (ptr) =3D qemu_alloc(sizeof(*= (ptr)))
#define=A0 QEMU_NEW_ARRAY(ptr,cnt)=A0=A0 (ptr) =3D qemu_calloc((cnt),sizeof= (*(ptr)))
#define=A0 QEMU_RENEW_ARRAY(ptr,cnt)=A0 (ptr) =3D qemu_realloc= ((ptr),(cnt),sizeof(*(ptr)))
#define=A0 QEMU_FREE_ARRAY(ptr)=A0 =A0=A0= =A0=A0=A0 qemu_free(ptr)

(yes, qemu_realloc() would take 3 parameters).

Any direct use of= malloc()/qemu_malloc() in source code would be suspicious and could
eas= ily spotted to check it.
=A0

J



--001636c5a435ee9dfe046b138882--