On 05/03/2014 01:59 PM, Cole Robinson wrote:
> RaiseErrorFull does not prepend the static error code string (like
> INVALID_ARG yields "invalid arg: %(msg)s"). We should be using
> ReportErrorHelper.
I'd get Dan's opinion on this one, since he first introduced the macros
in commit d91f3ef and may have had a reason not mentioned in that commit
message for preferring one form over the other.
>
> The generated error objects are slightly different, by not storing the
> invalid argument name in err->str2. However those fields aren't used
> anywhere else and aren't documented to contain anything useful, so
> I don't think it matters.
I think we haven't documented what the various fields of virError
contain precisely because documenting it would make it ABI, but the
whole object is already a nasty back-compat hack to pass over the wire.
The argument that it is undocumented is indeed the best supporting
argument that you can offer that making this change isn't likely to
break anything.
>
> # define virReportInvalidNullArg(argname) \
> - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \
> - VIR_FROM_THIS, \
> - VIR_ERR_INVALID_ARG, \
> - VIR_ERR_ERROR, \
> - __FUNCTION__, \
> - #argname, \
> - NULL, \
> - 0, 0, \
> - _("%s in %s must be NULL"), \
> - #argname, __FUNCTION__)
> + virReportErrorHelper(VIR_FROM_THIS, \
> + VIR_ERR_INVALID_ARG, \
> + __FILE__, __FUNCTION__, __LINE__, \
> + _("%s in %s must be NULL"), \
> + #argname, __FUNCTION__)
The transformation looks sane to me, but I'm reluctant to give ACK
without Dan weighing in on it.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list