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
