On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
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