From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1M6QYg-00059J-4N for qemu-devel@nongnu.org; Tue, 19 May 2009 10:44:46 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1M6QYa-00058b-M7 for qemu-devel@nongnu.org; Tue, 19 May 2009 10:44:44 -0400 Received: from [199.232.76.173] (port=45942 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1M6QYa-00058Y-Fb for qemu-devel@nongnu.org; Tue, 19 May 2009 10:44:40 -0400 Received: from mx2.redhat.com ([66.187.237.31]:53534) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1M6QYZ-0003g2-N7 for qemu-devel@nongnu.org; Tue, 19 May 2009 10:44:40 -0400 Date: Tue, 19 May 2009 11:44:24 -0300 From: Eduardo Habkost Subject: Re: [Qemu-devel] [PATCH] fix qemu_malloc() error check for size==0 Message-ID: <20090519144424.GD4254@blackpad> References: <1242678676-19439-1-git-send-email-ehabkost@redhat.com> <20090518221705.GO2079@blackpad> <8763fxvbfk.fsf@pike.pond.sub.org> <20090519140201.GB4254@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: Markus Armbruster , qemu-devel@nongnu.org On Tue, May 19, 2009 at 06:37:18PM +0400, malc wrote: > On Tue, 19 May 2009, Eduardo Habkost wrote: > > > On Tue, May 19, 2009 at 05:00:27PM +0400, malc wrote: > > > On Tue, 19 May 2009, Markus Armbruster wrote: > > > > > > > IOW making qemu_malloc[z] > > > > > return whatever the underlying system returns is just hiding the bugs, > > > > > the code becomes unportable. > > > > > > > > Matter of taste. > > > > > > > > 1. Deal with the implementation-definedness. Every caller that could > > > > pass zero needs to take care not to confuse empty allocation with an > > > > out of memory condition. > > > > > > > > This is easier than it sounds when you check for out of memory in > > > > just one place, like we do. > > > > > > > > 2. Remove the implementation-definedness. Easiest way is to detect zero > > > > size in a wrapper (for us: qemu_malloc()) and bump it to one. > > > > > > And mine: > > > 3. Abort the program if somebody tries it. Because so far history thought > > > me that nobody does 1. > > > > Are you sure about that? There may be cases where qemu_malloc(0) is > > called correctly, without the wrong assumptions about the returned > > value. > > > > You are proposing to make the qemu_malloc() API behavior diverge from > > the standard C malloc() behavior and prevent usage that is valid for > > malloc()/free() usage. Do you volunteer to audit all Qemu code to make > > sure the new behavior is safe? ;) > > That's the problem standard C does _not_ define the behaviour, and leaves > that to implementation. The only thing it doesn't define is either the returned pointer is NULL or not, and that doesn't make malloc(0) automatically unportable, because all the rest is perfectly defined: 1) You can't dereference the pointer (just like you can't dereference p[n] on a malloc(n) block) 2) You should pass the returned pointer to free() later > As for audit, that's precisely what aborting on > zero will try (and fail) to accomplish the offenders will be (given > unlimited time) caught. My point is that malloc(0) is not a bug, and I don't see a reason to make it an offense and diverge from standard malloc() and free(). > oom_check was added despite the fact that there > were places that correctly handled malloc's returning NULL. And i for > one do not know if there are/were places that called qemu_malloc with > zero and expected Linux behaviour. I agree that expecting the Linux behaviour (non-NULL) is a bug. My point is that there is no reason to consider malloc(0) a bug. -- Eduardo