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

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

Attachment: signature.asc
Description: OpenPGP digital signature


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