On Tue, Jan 15, 2013 at 05:30:23PM -0700, Eric Blake wrote:
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)
Yes, using a short in the struct would be better & was my intention.
+ 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()?
This function raises an error already.
+ + 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.
I thought this did too, but it appears to leave it upto the caller, so I'll add it here. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|