Re: [libvirt] [PATCH] [v2] API: Improve log for domain related APIs

On 01/05/2011 03:40 AM, Osier Yang wrote: >> Then maybe the patch should be altered to output both name and UUID >> (probably by introducing a helper function, which when given a >> virDomainPtr outputs all three pieces of debug information rather than >> the current %p). I like the idea behind the patch, but only if we can >> come up with the correct way of getting the extra information when >> debugging is enabled, and without adding extra time when debugging is >> off. I'm not even sure what the right internal APIs for the job would >> be, or if they even exist yet. >> > > Hi, Eric, > > I implemented a helper function like so in src/libvirt.c: > > static const char * > virDomainNameUUID(virDomainPtr domain) { > char *entry = NULL; > const char *name = NULL; > char uuidstr[VIR_UUID_STRING_BUFLEN]; > > if (!VIR_IS_DOMAIN(domain)) { > entry = strdup("(VM: invalid domain)"); malloc'd return... > return entry; > } > > name = domain->name; > virUUIDFormat(domain->uuid, uuidstr); > > if(virAsprintf(&entry, "(VM: name=%s, uuid=%s)", name, > uuidstr) < 0) > virReportOOMError(); > > return NULLSTR(entry); and static return. Ouch - that means the caller doesn't know whether to call free or not. You have to be consistent (the result is either NULL or malloc'd; or the result is always static). > } > > But how to avoid extra time when debugging is off? Following > is not correct way definitely, as malloc'ed "entry" need to be > freed. > > virConnectPtr > virDomainGetConnect (virDomainPtr dom) > { > DEBUG("dom=%p, %s", dom, virDomainNameUUID(dom)); When I envisioned a helper function, I'm thinking more like callers using a simpler: DEBUG_DOMAIN(dom) and a helper like: void DEBUG_DOMAIN (virDomainPtr dom) { if !DEBUG then early exit gather data DEBUG("dom=%p, %s", dom, virDomainNameUUID(dom)); free any gathered data if needed } That would mean that your new helper function would need to look at the guts of DEBUG to mirror the same decision of whether DEBUG will result in any output.