[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH v9] block/raw-posix.c: Make physica

From: Eric Blake
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH v9] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host
Date: Mon, 30 Nov 2015 09:49:35 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0

On 11/30/2015 09:38 AM, Programmingkid wrote:

>>>>>> +    /* if a working partition on the device was not found */
>>>>>> +    if (partition_found == false) {
>>>>>> +        error_setg(errp, "Error: Failed to find a working partition on "
>>>>>> +                                                                     
>>>>>> "disc!\n");
>>>>> and I already pointed out on v8 that this is not the correct usage of
>>>>> error_setg().  So, here's hoping v10 addresses the comments here and
>>>>> elsewhere.
>>>> Kevin Wolf wanted it this way. What would you do instead?
>>> Keven and I both want you to use error_setg(), but to use it correctly -
>>> and the correct way is to NOT supply a trailing \n.
>> Nor leading "Error:", for that matter.
> I just think that using "Error" does communicate the fact that something is 
> wrong
> a lot better than just printing the message. 

But error_setg() _already_ provides the context that an error message is
being printed.  The whole point of using wrapper functions is that the
common functionality (like an 'error:' prefix, or '\n' suffix) is done
in the wrapper, not at every call site.  If you were using raw printf(),
then yes, using your own 'Error:' prefix would be appropriate.  But we
aren't using raw printf().  Your use of an 'Error:' prefix is therefore
redundant, and we are trying to convince you that you are using
error_setg() incorrectly.

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]