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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list