Re: [Qemu-devel] [PATCH v9] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Eric Blake]

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

signature.asc
Description: OpenPGP digital signature