Re: [Qemu-devel] [RFC 00/14]: add printf-like human msg to error_set()

[Markus Armbruster]

"Daniel P. Berrange" <address@hidden> writes:

> On Wed, Jul 25, 2012 at 09:43:55PM -0500, Anthony Liguori wrote:
>> Luiz Capitulino <address@hidden> writes:
>> 
>> > Basically, this series changes a call like:
>> >
>> >  error_set(errp, QERR_DEVICE_NOT_FOUND, device);
>> >
>> > to:
>> >
>> >  error_set(errp, QERR_DEVICE_NOT_FOUND,
>> >            "Device 'device=%s' not found", device);
>> >
>> > In the first call, QERR_DEVICE_NOT_FOUND is a string containing a json 
>> > dict:
>> >
>> >     "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
>> 
>> This is the wrong direction.  Looking through the patch, this makes the
>> code much more redundant overall.  You have dozens of calls that are
>> duplicating the same error message.  This is not progress.
>
> What we do in libvirt, is to define helper functions for
> reporting the specific error codes. So, as well as having
> the generic
>
>   error_set(errp, QERR_CODE, "format string", args)
>
> you would have
>
>   error_set_device_not_found(errp, args)
>
> which is just a #define
>
>   #define error_set_device_not_found(errp, args) \
>      error_set(errp, QERR_DEVICE_NOT_FOUND, "Device %s not found", args)
>
> for the most part this should avoid the duplication you're concerned
> about, while still letting use provided more detailed messsages.

Yup.  I suggested the same on IRC, modulo macro vs. function.