On Tue, Apr 29, 2008 at 04:42:54PM +0200, Soren Hansen wrote:
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.
Here is an update to the HACKING file intended to describe some of the conventions in use... Dan Index: HACKING =================================================================== RCS file: /data/cvs/libvirt/HACKING,v retrieving revision 1.1 diff -r1.1 HACKING 45a46,160
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); }
-- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|