Re: [libvirt] [PATCH 2/5] virerror: Fix incorrect use of RaiseErrorFull
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
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)