On Thu, May 28, 2009 at 2:47 PM, Doug Goldstein <cardoe@xxxxxxxxxx> wrote: > On Thu, May 28, 2009 at 9:48 AM, Daniel P. Berrange <berrange@xxxxxxxxxx> wrote: >> On Thu, May 28, 2009 at 09:04:55AM -0500, Doug Goldstein wrote: > > <snip> > >> >> I don't much like this function with the mix of fixe length buffers, >> and fixed length malloc()'s. I think it'd be better to split out the >> code for breaking $PATH into a list of strings into a separate function, >> eg >> >> int virSplitPath(const char *src, const char **dst); >> >> So it'd take the $PATH value, and return a list of strings in 'dst', and >> the number of strins as the return value. >> >> Then the virFindFileInPath() method, would be better to iterate over >> this, and use virAsprintf() to build the full path, rather than >> relying on fixed size buffers. >> > > The point of using the fixed length buffer is for memory > fragmentation. While I will agree that it may be confusing at first > glance, the point is that libvirtd is a long running daemon and in a > short term scope a little extra memory usage is acceptable than to > fragment the memory space slowly over time. Since the scope of the > first variable is just the scope of the function, a fixed length > buffer is simply just some room on the stack and its gone once the > function is out of scope. The return value is just allocated once and > returned. > > Implementing the pure allocation method as described above would > result in an unnecessary amount of small allocations. One for the char > array. One per each piece of path (on Fedora 10 there are 10 > components to a default install in $PATH). Then one for path piece + > binary name. This would result in 21 allocations per call. That's a > lot of memory churn for a short running piece of code. > > -- > Doug Goldstein > Here's v2 of the patch. Changed to use goto and VIR_ALLOC/FREE -- Doug Goldstein
Attachment:
libvirt-0.6.3-kvm-img.patch
Description: Binary data
-- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list