On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> Introduce a virPortAllocator for managing TCP port allocations.
> ---
> .gitignore | 1 +
> src/Makefile.am | 1 +
> src/libvirt_private.syms | 6 ++
> src/util/virportallocator.c | 188 +++++++++++++++++++++++++++++++++++++++++
> src/util/virportallocator.h | 40 +++++++++
> tests/Makefile.am | 17 +++-
> tests/virportallocatortest.c | 195 +++++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 447 insertions(+), 1 deletion(-)
> create mode 100644 src/util/virportallocator.c
> create mode 100644 src/util/virportallocator.h
> create mode 100644 tests/virportallocatortest.c
>
> +
> + 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.
> +++ b/tests/Makefile.am
> @@ -97,6 +97,7 @@ test_programs = virshtest sockettest \
> virbitmaptest \
> virlockspacetest \
> virstringtest \
> + virportallocatortest \
Space vs. tab issue (one of the few files where we prefer tab).
> +
> +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/libvirportallocatormock.so")
Nice way to fake out bind() - this PRELOAD test idiom is turning out to
be useful.
ACK if you address the points above.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list