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 -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list