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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1551 bytes --]

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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]