Re: [Qemu-devel] [PATCH v2 2/2] ivshmem: add missing error exit(2)

["Andreas Färber" <afaerber@suse.de>]

Am 03.06.2013 15:11, schrieb Stefan Hajnoczi:
> On Mon, Jun 03, 2013 at 06:26:22AM -0600, Eric Blake wrote:
>> On 06/03/2013 02:58 AM, Stefan Hajnoczi wrote:
>>> If the user fails to specify 'chardev' or 'shm' then we cannot continue.
>>> Exit right away so that we don't invoke shm_open(3) with a NULL pointer.
>>>
>>> It would be nice to replace exit(1) with error returns in the PCI device
>>> .init() function, but leave that for another patch since exit(1) is
>>> currently used elsewhere.
>>>
>>> Spotted by Coverity.
>>>
>>> Cc: Cam Macdonell <cam@cs.ualberta.ca>
>>> Cc: qemu-stable@nongnu.org
>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> ---
>>>  hw/misc/ivshmem.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
>>> index a19a6d6..5658f73 100644
>>> --- a/hw/misc/ivshmem.c
>>> +++ b/hw/misc/ivshmem.c
>>> @@ -735,6 +735,7 @@ static int pci_ivshmem_init(PCIDevice *dev)
>>>  
>>>          if (s->shmobj == NULL) {
>>>              fprintf(stderr, "Must specify 'chardev' or 'shm' to ivshmem\n");
>>> +            exit(1);
>>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>
>> However, I prefer exit(EXIT_FAILURE) rather than exit(1), to make it a
>> bit easier to grep for known failure exits.  Libvirt has a syntax
>> checker (taken from gnulib) that enforces such a style, if qemu would
>> like to adopt that style.
> 
> I like EXIT_FAILURE too but the rest of the file inconsistently uses
> exit(1) and exit(-1).  I decided exit(1) was the least evil but the
> longer term fix is to return instead of exiting.

QOM realize would solve that, but similar to virtio the PCI qdev initfn
actually does something (allocating the config region among others) so
faces the same design questions of how to best inherit QOM methods as
currently being discussed. Shouldn't hold up a bug fix like this one.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg