Re: [libvirt] [PATCH v3] leaseshelper: improvements to support all events
On 09/11/2014 10:21 AM, Nehal J Wani wrote:
>
> Here you could just go through the json array and format each lease.
I considered that, but then since the requirement of DNSMASQ is to
print ipv4 leases first and then ipv6 leases, I would have to iterate
over the JSON array twice! (Writing to the file would be a third
iteration). I thought the extra space wouldn't matter as long as we
are not increasing the runtime. Also, I am only copying the pointer to
the leases in the JSON array, and not the complete lease itself. Do
you still think this is a bad idea?
Going through the array twice is not a problem algorithmically (that's
still O(n), with just a slightly larger constant). Sorting, on the
other hand, is O(n log n) minimum complexity. Sometimes, simplicity of
multiple loops outweighs complexity of trying to optimize prematurely.
>
> Otherwise looks good.
>
> Peter
>
Thanks for reviewing the patch. If we decide to remove the 'pointer
copy' stuff, then I'll send v4. If not, then I'll handle the OOM cases
and then send v4.
At this point, I don't have any strong opinion on which approach is
better, so go ahead and send v4 with the version you like.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 539 bytes)