[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2] Implement DNS SRV record into the bridge driver



On 08/11/2011 02:51 PM, Laine Stump wrote:

+
+ /* Following attributes are optional but we had to make sure their
NULL above */

s/their/they're/

Right, because VIR_REALLOC_N doesn't 0-fill the new memory (it doesn't
know how many there were before).

But VIR_EXPAND_N and VIR_RESIZE_N _do_ know how to 0-fill memory; those are better macros to use when resizing an array where you want to keep initial contents, and avoid having to repeat 0-initialization yourselves (which macro depends on how large you ever expect the array to grow).

+
+ if (def->srvrecords[i].domain)
+ virBufferAsprintf(buf, "domain='%s' ", def->srvrecords[i].domain);
+ if (def->srvrecords[i].target)
+ virBufferAsprintf(buf, "target='%s' ", def->srvrecords[i].target);
+ if (def->srvrecords[i].port)
+ virBufferAsprintf(buf, "port='%d' ", def->srvrecords[i].port);
+ if (def->srvrecords[i].priority)
+ virBufferAsprintf(buf, "priority='%d' ", def->srvrecords[i].priority);
+ if (def->srvrecords[i].weight)
+ virBufferAsprintf(buf, "weight='%d' ", def->srvrecords[i].weight);

Since 0 is a valid value for these according to the RFC, you should make
their defaults -1 (see above where they're initialized), and check for
-1 here.

and/or keep separate bool or bitfields tracking which fields were explicitly set (even if set to 0); the advantage of separate bools is that struct 0-initialization then gets your bool in the right state, instead of you having to manually set things to -1 for unset.

--
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]