From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1M6PkL-00034q-4M for qemu-devel@nongnu.org; Tue, 19 May 2009 09:52:45 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1M6PkG-0002xz-Dw for qemu-devel@nongnu.org; Tue, 19 May 2009 09:52:44 -0400 Received: from [199.232.76.173] (port=41056 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1M6PkG-0002xJ-1o for qemu-devel@nongnu.org; Tue, 19 May 2009 09:52:40 -0400 Received: from mx2.redhat.com ([66.187.237.31]:55150) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1M6PkE-0001sW-9E for qemu-devel@nongnu.org; Tue, 19 May 2009 09:52:38 -0400 Date: Tue, 19 May 2009 10:52:16 -0300 From: Eduardo Habkost Subject: Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0 Message-ID: <20090519135216.GA4254@blackpad> References: <1242678676-19439-1-git-send-email-ehabkost@redhat.com> <20090518221705.GO2079@blackpad> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: malc Cc: qemu-devel@nongnu.org On Tue, May 19, 2009 at 04:17:55AM +0400, malc wrote: > On Mon, 18 May 2009, Eduardo Habkost wrote: > > > On Tue, May 19, 2009 at 01:56:55AM +0400, malc wrote: > > > On Mon, 18 May 2009, Eduardo Habkost wrote: > > > > > > > This patch is similar to a previous qemu_realloc() fix > > > > (commit 322691a5c9f1c8531554148d47c078b5be590805), but for qemu_malloc(). > > > > > > > > malloc(0) may correctly return NULL if size==0. We don't want to abort qemu on > > > > this case. > > > > > > Only it wouldn't (on Linux): > > > > > > $ cat malloc.c > > > #include > > > > > > int main (void) > > > { > > > printf ("%p\n", malloc (0)); > > > return 0; > > > } > > > $ gcc malloc.c > > > $ ./a.out > > > 0x10011008 > > > > > > Standard (in 7.20.3) says that malloc's behaviour in case of size being > > > zero is implementation defined. > > > > > > Try `git show 63c75dcd669d011f438421980b4379827da4bb1c'. > > > > > > The best(only?) thing to do is to check size passed to qemu_malloc[z] > > > and abort the program if this situation is encountered. > > > > Why? malloc(0) is as valid as realloc(p, 0). It will either return NULL > > or a pointer, and on any case the value can be safely passed to free() > > later. > > I believe you haven't examined the commit that i referenced. Thing is > existing code used to, i'd venture a guess accidentaly, rely on the > behaviour that current GLIBC provides and consequently failed to > operate on AIX where malloc(0) returns NULL, IOW making qemu_malloc[z] > return whatever the underlying system returns is just hiding the bugs, > the code becomes unportable. The assumption that malloc(0) will return anything (either NULL or not-NULL) is not portable. That's exactly the point of my patch: not making any assumption about the returned value when size==0. But calling malloc(0) is perfectly valid, as long as you call free() on the returned value later. I don't see any reason to make the qemu_malloc() behavior from the standard malloc() behavior. The sequence "p=malloc(0);free(p)" is valid and works. Why would we prevent "p=qemu_malloc(0);qemu_free(p)" from working? Yes, we may have broken code that assumes that qemu_malloc(0) is not NULL, and _that_ code is broken and must be fixed. But why would we break the cases where qemu_malloc(0) is called and handled correctly? -- Eduardo