[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]