Re: [libvirt] [PATCH 5/7] Add a port allocator class

On 01/11/2013 05:13 AM, Daniel P. Berrange wrote: > + > + unsigned int start; > + unsigned int end; > +}; > + > +virPortAllocatorPtr virPortAllocatorNew(unsigned short start, > + unsigned short end) > + > +{ Spurious blank line. Any reason you allocate with short, but store the values in int internally? (unsigned short in the struct should be fine)

> + virPortAllocatorPtr pa; > + > + if (start >= end) { > + virReportInvalidArg(start, "start port %d must be less than end port %d", > + start, end); > + return NULL; > + } Since this error gave a message, > + > + if (virPortAllocatorInitialize() < 0) > + return NULL; > + > + if (!(pa = virObjectLockableNew(virPortAllocatorClass))) > + return NULL; does this error need to call virReportOOMError()?

> + > + pa->start = start; > + pa->end = end; > + > + if (!(pa->bitmap = virBitmapNew(end-start))) { > + virObjectUnref(pa); > + return NULL; Same question here. Callers can't tell if a NULL return means OOM or usage error.