Re: [Libvir] [PATCH] Add bus attribute to disk target definition

On Tue, Apr 29, 2008 at 03:17:14PM +0100, Daniel P. Berrange wrote: >>>> + if (!bus) >>>> + disk->bus = QEMUD_DISK_BUS_IDE; >>>> + else if (!strcmp((const char *)bus, "ide")) >>>> + disk->bus = QEMUD_DISK_BUS_IDE; >>>> + else if (!strcmp((const char *)bus, "scsi")) >>>> + disk->bus = QEMUD_DISK_BUS_SCSI; >>>> + else if (!strcmp((const char *)bus, "virtio")) >>>> + disk->bus = QEMUD_DISK_BUS_VIRTIO; >>> Can you use the STREQ macro here instead of strcmp. >> Erm... I *could*.. I'm curious, though, why e.g. the similar code right >> above it doesn't use STREQ if that's the preferred way to do it? > We've been slowly updating code to match these new standards when doing > patches. Well, if that's the way you do it, I'll follow suit.. However, I have to say that I pity the person that reads the code and finds these two sections of code that seem to do rather similar things, but use different functions to do it, and then has to work out what on earth the difference between the two might be.

Low level memory management =========================== Use of the malloc/free/realloc/calloc APIs is deprecated in the libvirt codebase, because they encourage a number of serious coding bugs and do not enable compile time verification of checks for NULL. Instead of these routines, use the macros from memory.h - eg to allocate a single object: virDomainPtr domain; if (VIR_ALLOC(domain) < 0) { __virRaiseError(VIR_ERROR_NO_MEMORY) return NULL; } - eg to allocate an array of objects virDomainPtr domains; int ndomains = 10; if (VIR_ALLOC_N(domains, ndomains) < 0) { __virRaiseError(VIR_ERROR_NO_MEMORY) return NULL; } - eg to allocate an array of object pointers virDomainPtr *domains; int ndomains = 10; if (VIR_ALLOC_N(domains, ndomains) < 0) { __virRaiseError(VIR_ERROR_NO_MEMORY) return NULL; } - eg to re-allocate the array of domains to be longer ndomains = 20 if (VIR_REALLOC_N(domains, ndomains) < 0) { __virRaiseError(VIR_ERROR_NO_MEMORY) return NULL; } - eg to free the domain VIR_FREE(domain); String comparisons ================== Do not use the strcmp, strncmp, etc functions directly. Instead use one of the following semantically named macros - For strict equality: STREQ(a,b) STRNEQ(a,b) - For case sensitive equality: STRCASEEQ(a,b) STRCASENEQ(a,b) - For strict equality of a substring: STREQLEN(a,b,n) STRNEQLEN(a,b,n) - For case sensitive equality of a substring: STRCASEEQLEN(a,b,n) STRCASENEQLEN(a,b,n) - For strict equality of a prefix: STRPREFIX(a,b) Variable length string buffer ============================= If there is a need for complex string concatenations, avoid using the usual sequence of malloc/strcpy/strcat/snprintf functions and make use of the virBuffer API described in buf.h eg typical usage is as follows: char * somefunction(...) { virBuffer buf = VIR_BUFFER_INITIALIZER; ... virBufferAdd(&buf, "<domain>\n"); virBufferVSprint(&buf, " <memory>%d</memory>\n", memory); ... virBufferAdd(&buf, "</domain>\n"); .... if (virBufferError(&buf)) { __virRaiseError(...); return NULL; } return virBufferContentAndReset(&buf); }