From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MBYK8-0005TT-KU for qemu-devel@nongnu.org; Tue, 02 Jun 2009 14:02:56 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MBYK4-0005Pe-OK for qemu-devel@nongnu.org; Tue, 02 Jun 2009 14:02:56 -0400 Received: from [199.232.76.173] (port=39403 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MBYK3-0005NQ-Gk for qemu-devel@nongnu.org; Tue, 02 Jun 2009 14:02:52 -0400 Received: from smtp-out.google.com ([216.239.45.13]:14354) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1MBYK1-00069J-9m for qemu-devel@nongnu.org; Tue, 02 Jun 2009 14:02:50 -0400 Received: from zps75.corp.google.com (zps75.corp.google.com [172.25.146.75]) by smtp-out.google.com with ESMTP id n52I2iav010113 for ; Tue, 2 Jun 2009 11:02:44 -0700 Received: from fxm19 (fxm19.prod.google.com [10.184.13.19]) by zps75.corp.google.com with ESMTP id n52I2WM0007549 for ; Tue, 2 Jun 2009 11:02:43 -0700 Received: by fxm19 with SMTP id 19so8440326fxm.46 for ; Tue, 02 Jun 2009 11:02:42 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4A24D40E.2060704@redhat.com> References: <200905290758.11551.jcd@tribudubois.net> <4A1FD6E2.9020006@redhat.com> <200905291407.26757.paul@codesourcery.com> <200905291917.10535.jseward@acm.org> <60cad3f0905291412m670c7a6cw45f9b51f3122ddfb@mail.gmail.com> <4A24D40E.2060704@redhat.com> Date: Tue, 2 Jun 2009 20:02:40 +0200 Message-ID: <60cad3f0906021102o635b05f7m91593060683bb338@mail.gmail.com> Subject: Re: [Qemu-devel] [PATCH] use qemu_malloc and friends consistently From: David Turner Content-Type: multipart/alternative; boundary=001636c599a345956e046b615938 List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: Kevin Wolf , Paul Brook , qemu-devel@nongnu.org, Jean-Christophe Dubois --001636c599a345956e046b615938 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On Tue, Jun 2, 2009 at 9:26 AM, Gerd Hoffmann wrote: > On 05/29/09 23:12, David Turner wrote: > >> 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) >> > > The idea to have allocators for arrays (and have them allow zero-length > arrays) is fine. I wouldn't create two macros for new and renew array, you > can just use usual realloc semantics (ptr == NULL -> alloc). > > > Also I don't like the syntax that much as you'll have the IMHO > non-intuitive code like this: > > QEMU_NEW_ARRAY(ptr, ...); > > instead of > > ptr = QEMU_NEW_ARRAY(...); > > then. I don't see another easy way to get the automagic sizeof(*ptr) stuff > done though. > The first version will extract the item size automatically for you, making it less likely that you screw things in the second version's parameter list. But I'm not going to fight against it. Also, my code is usually a lot more aggressive than what I proposed. I don't want to burn too many cycles on this, just stating that, generally speaking, it is possible to use macros/wrappers to both make the code's intent more clear and reduce potential errors. I feel this much more worthwhile than imposing an abort on qemu_malloc(0), which seems quite arbitrary, as discussed heavily on this thread. Apart from that, the exact details on how to achieve the above goal don't matter much. > > cheers, > Gerd > > --001636c599a345956e046b615938 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

On Tue, Jun 2, 2009 at 9:26 AM, Gerd Hof= fmann <kraxel@red= hat.com> wrote:
On 05/29/09 23:12, David Turner wrote:
I would even suggest providing helper macros to make the programmer's i= ntent
even more clear
and less error-prone, as in:

#define =A0QEMU_NEW(ptr) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(ptr) =3D q= emu_alloc(sizeof(*(ptr)))
#define =A0QEMU_NEW_ARRAY(ptr,cnt) =A0 (ptr) =3D qemu_calloc((cnt),sizeof(*= (ptr)))
#define =A0QEMU_RENEW_ARRAY(ptr,cnt) =A0(ptr) =3D
qemu_realloc((ptr),(cnt),sizeof(*(ptr)))
#define =A0QEMU_FREE_ARRAY(ptr) =A0 =A0 =A0 =A0qemu_free(ptr)

The idea to have allocators for arrays (and have them allow zero-length arr= ays) is fine. =A0I wouldn't create two macros for new and renew array, = you can just use usual realloc semantics (ptr =3D=3D NULL -> alloc).
=


=A0

Also I don't like the syntax that much as you'll have the IMHO non-= intuitive code like this:

=A0QEMU_NEW_ARRAY(ptr, ...);

instead of

=A0ptr =3D QEMU_NEW_ARRAY(...);

then. =A0I don't see another easy way to get the automagic sizeof(*ptr)= stuff done though.

The first version w= ill extract the item size automatically for you, making it less likely that= you screw things in the second version's parameter list.
But I'm not going to fight against it.

Al= so, my code is usually a lot more aggressive than what I proposed. I don= 9;t want to burn too many cycles on this, just stating that,
generally speaking, it is possible to use macros/wrappers to both make the = code's intent more clear and reduce potential errors.

I feel this much more worthwhile than imposing an abort on qemu_mal= loc(0), which seems quite arbitrary, as discussed heavily on this thread.
=A0Apart from that, the exact details on how to achieve=A0the above go= al don't matter much.

=A0

cheers,
=A0Gerd


--001636c599a345956e046b615938--