qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Avi Kivity <avi@redhat.com>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: qemu-devel@nongnu.org, Paul Brook <paul@codesourcery.com>,
	Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() & friends
Date: Sat, 05 Dec 2009 20:06:08 +0200	[thread overview]
Message-ID: <4B1AA110.8030600@redhat.com> (raw)
In-Reply-To: <4B1A9E39.2030602@codemonkey.ws>

On 12/05/2009 07:54 PM, Anthony Liguori wrote:
> Avi Kivity wrote:
>> A zero-supporting qemu_malloc() is fully compatible with malloc(), 
>> we're only restricting the possible returns.  So we're not misleading 
>> any caller.  In fact, taking your argument to the extreme, a malloc 
>> implementation would need to
>
> This is really the crux of the whole argument.  You're arguing that 
> while most people rely on incorrect idioms with malloc(), the problem 
> is not the idioms themselves but the definition of malloc().  The 
> opposing argument is that instead of providing a "fixed" version of 
> malloc(), we should encourage people to use a proper idiom.

When we see a lengthy and error prone idiom we usually provide a 
wrapper.  That wrapper is qemu_malloc().  If you like, don't see it as a 
fixed malloc(), but as qemu's way of allocating memory which is totally 
independent from malloc().

>
> I dislike the entire notion of qemu_malloc().  I've always disliked 
> the fact that it abort()s on OOM.  I'd rather see us use a normal 
> malloc() and code to that malloc currently which means avoiding size=0 
> and checking NULL results.

I believe that's impossible.  Many times, when the guest writes to a 
register, you have no way to indicate failure.  Sometimes you can 
indicate failure (say, time out on IDE write requests) but will likely 
cause much damage to the guest.  Preallocating memory is likely to be 
very difficult and also very wasteful (since you'll have to account for 
the worst case).

Furthermore, Linux under its default configuration will never fail a 
small allocation, instead oom-killing a semi-random process.  So all 
that work will not help on that platform.

>
> However, this is all personal preference and I'd rather focus my 
> energy on things that have true functional impact.  Markus raised a 
> valid functional problem with the current implementation and I 
> proposed a solution that would address that functional problem.  I'd 
> rather see the discussion focus on the merits of that solution than 
> revisiting whether ANSI got the semantics of malloc() correct in the 
> standards definition.
>

Unless ANSI has a say on qemu_malloc(), I think it's worthwhile to get 
that right rather than wrapping every array caller with useless tests.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

  reply	other threads:[~2009-12-05 18:06 UTC|newest]

Thread overview: 119+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-30 13:55 [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() & friends Markus Armbruster
2009-11-30 14:01 ` Avi Kivity
2009-11-30 14:23 ` Kevin Wolf
2009-12-01 12:40 ` Gerd Hoffmann
2009-12-01 12:57   ` Paul Brook
2009-12-01 13:47     ` Glauber Costa
2009-12-01 14:08       ` Markus Armbruster
2009-12-01 14:47         ` Gerd Hoffmann
2009-12-01 14:21       ` Paul Brook
2009-12-01 12:57   ` Gerd Hoffmann
2009-12-01 13:11   ` Markus Armbruster
2009-12-01 14:34   ` Avi Kivity
2009-12-01 14:53     ` Gerd Hoffmann
2009-12-01 15:32 ` Eduardo Habkost
2009-12-04 16:49 ` Anthony Liguori
2009-12-05 13:55   ` Markus Armbruster
2009-12-05 14:14     ` Laurent Desnogues
2009-12-05 17:08     ` malc
2009-12-05 17:23       ` Avi Kivity
2009-12-05 18:30       ` Reimar Döffinger
2009-12-06  7:57       ` Markus Armbruster
2009-12-06  8:39         ` malc
2009-12-06  8:59           ` Markus Armbruster
2009-12-06 10:22             ` malc
2009-12-06 10:40               ` Avi Kivity
2009-12-06 11:53                 ` malc
2009-12-06 12:07                   ` Avi Kivity
2009-12-06 12:11                     ` malc
2009-12-06 12:23                       ` Avi Kivity
2009-12-06 11:10               ` Markus Armbruster
2009-12-06 12:00                 ` malc
2009-12-06 16:23                   ` [Qemu-devel] " Paolo Bonzini
2009-12-07  8:35                   ` [Qemu-devel] " Kevin Wolf
2009-12-07  9:42                   ` Markus Armbruster
2009-12-07 10:00                     ` malc
2009-12-07 10:17                       ` Kevin Wolf
2009-12-07 10:35                       ` Markus Armbruster
2009-12-06 11:35               ` [Qemu-devel] " Paolo Bonzini
2009-12-06 12:02                 ` malc
2009-12-06 16:23                   ` Paolo Bonzini
2009-12-06  9:02           ` [Qemu-devel] " Blue Swirl
2009-12-06 10:02             ` malc
2009-12-05 17:07   ` Avi Kivity
2009-12-05 17:27     ` Anthony Liguori
2009-12-05 17:40       ` Avi Kivity
2009-12-05 17:54         ` Anthony Liguori
2009-12-05 18:06           ` Avi Kivity [this message]
2009-12-05 20:58             ` Anthony Liguori
2009-12-05 22:26               ` Avi Kivity
2009-12-06  8:24                 ` Markus Armbruster
2009-12-06 18:36                 ` Jamie Lokier
2009-12-06  8:12       ` Markus Armbruster
2009-12-06 16:52         ` Ian Molton
2009-12-06 17:14           ` Avi Kivity
2009-12-06 17:45             ` malc
2009-12-06 18:02               ` Avi Kivity
2009-12-06 18:12                 ` malc
2009-12-06 18:19                   ` Avi Kivity
2009-12-06 18:41                     ` malc
2009-12-07  9:47                       ` Avi Kivity
2009-12-07 10:20                         ` Kevin Wolf
2009-12-06 22:38                 ` Ian Molton
2009-12-07  2:51                   ` Jamie Lokier
2009-12-07  9:39                     ` Ian Molton
2009-12-07  9:55                       ` [Qemu-devel] " Paolo Bonzini
2009-12-07 13:28                         ` Avi Kivity
2009-12-07  9:45           ` [Qemu-devel] " Markus Armbruster
2009-12-07  8:48       ` Kevin Wolf
2009-12-07 17:32       ` Glauber Costa
2009-12-05 17:28     ` Blue Swirl
2009-12-05 17:44       ` Avi Kivity
2009-12-05 18:16         ` Laurent Desnogues
2009-12-05 23:08         ` Ian Molton
2009-12-05 23:11           ` Avi Kivity
2009-12-05 23:25             ` Ian Molton
2009-12-06 13:07               ` Avi Kivity
2009-12-06 16:58                 ` Ian Molton
2009-12-06 17:07                   ` Avi Kivity
2009-12-06 17:47                     ` malc
2009-12-06 17:59                       ` Avi Kivity
2009-12-06 18:09                         ` malc
2009-12-06 18:16                           ` Avi Kivity
2009-12-06 18:21                             ` malc
2009-12-06 22:40                           ` Ian Molton
2009-12-06 18:31               ` Jamie Lokier
2009-12-07  9:56                 ` Markus Armbruster
2009-12-07 11:30 ` malc
2009-12-07 14:45   ` Markus Armbruster
2009-12-07 16:55     ` malc
2009-12-08  8:21       ` Markus Armbruster
2009-12-08 10:22         ` malc
2009-12-07 15:50 ` Anthony Liguori
2009-12-07 16:00   ` Avi Kivity
2009-12-07 16:06     ` Anthony Liguori
2009-12-07 16:11       ` Avi Kivity
2009-12-07 16:20         ` Anthony Liguori
2009-12-07 16:26           ` Avi Kivity
2009-12-07 16:32             ` Anthony Liguori
2009-12-07 16:37               ` Avi Kivity
2009-12-07 16:59                 ` Anthony Liguori
2009-12-07 17:07                   ` Avi Kivity
2009-12-07 17:09                     ` Anthony Liguori
2009-12-07 17:13                       ` Avi Kivity
2009-12-07 17:17                         ` Anthony Liguori
2009-12-07 17:19                           ` Avi Kivity
2009-12-07 17:40                             ` Anthony Liguori
2009-12-07 18:25                               ` Avi Kivity
2009-12-07 18:59                                 ` Anthony Liguori
2009-12-07 19:01                                   ` Avi Kivity
2009-12-07 19:07                                     ` Anthony Liguori
2009-12-07 16:24   ` Paul Brook
2009-12-07 16:27     ` Anthony Liguori
2009-12-07 16:28     ` Avi Kivity
2009-12-07 16:57   ` malc
2009-12-07 17:01     ` Anthony Liguori
2009-12-07 17:09       ` malc
2009-12-08  9:02         ` Kevin Wolf
2009-12-07 18:12   ` Blue Swirl
2009-12-08  8:30   ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B1AA110.8030600@redhat.com \
    --to=avi@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=armbru@redhat.com \
    --cc=paul@codesourcery.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).