On 12/19/2013 09:37 AM, Daniel P. Berrange wrote: >>>> +#define virLibDomainSnapshotError(code, ...) \ >>>> + virReportErrorHelper(VIR_FROM_DOMAIN_SNAPSHOT, code, __FILE__, \ >>>> + __FUNCTION__, __LINE__, __VA_ARGS__) >>> >>> I'd venture to sugggest that these functions really have little >>> to no benefit / reason to exist. They aren't really simplifying >>> like, just making it different. They're historical baggage from >>> the old time when they were actual functions which did extra >>> sprintf formatting work. Could we just remove them ?? >> >> Perhaps, by using virReportError instead. I can give that a try; but >> just looking at it, virReportError() hardcodes VIR_FROM_THIS as its >> first argument, whereas virLib*Error() sets different VIR_FROM_* >> categories. Worse, we use multiple calls from within a single function >> (so continually redefining VIR_FROM_THIS would get painful) - for >> example, virDomainSnapshotCreateXML() uses both virLibDomainError() >> (VIR_FROM_DOMAIN) and virLibConnError() (VIR_FROM_NONE). > > Oh true I'd forgotten it relied on VIR_FROM_THIS. > > How about just renaming them s/Lib/Report/ > eg virLibDomainError -> virReportDomainError and having > them in datatypes.h too. Sounds reasonable, but again, splitting into two patches to keep the review work easier. v2 coming up later today. -- 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