On Fri, Feb 27, 2015 at 08:58:04AM -0800, Shawn Landden wrote: > On Thu, Feb 26, 2015 at 6:22 PM, Zbigniew Jędrzejewski-Szmek < > zbys...@in.waw.pl> wrote: > > > On Thu, Feb 26, 2015 at 08:04:08AM +0000, Jan Janssen wrote: > > > Shawn Landden <shawn <at> churchofgit.com> writes: > > > > > > > void strv_free(char **l) { > > > > - strv_clear(l); > > > > + char **k; > > > > + > > > > + if (!l) > > > > + return; > > > > + > > > > + for (k = l; *k; k++) > > > > + free(*k); > > > > + > > > > free(l); > > > > } > > > What are you trying to achieve here? I see no point in optimizing out > > the *l > > > = NULL from strv_clear. > > > > > > > + entry->linux_loc = l + strspn(l, > > > WHITESPACE); > > > > + else if ((l = startswith(m, "initrd "))) > > > > + entry->initrd = l + strspn(l, > > > WHITESPACE); > > > You need to support more than one initrd per kernel, see > > > https://wiki.archlinux.org/index.php/Microcode for why. Also, I am > > pretty > > > sure you can have a initrd=/path/to/initrd in the kernel options entry. > > > Since the efi bootloader just appends each given initrd to the kernel > > > command line. > > > > > > > > > All in all I am wondering why you need a rbtree for all this in the first > > > place? A simple hashmap should do just fine. > > A hashmap does not keep order. But a simple array + qsort_safe() should > > work too. I'm wary of introducing yet another data structure into systemd > > which raises the bar for people editing the code later on or making > > changes. > > > > We need two operations: sorting kernels to list them, and picking (I > > presume) the > > latest kernel or the kernel with the given version. Both of those > > operations > > are done once over the lifetime of the program, so any speedup in using > > a data structure should take into account the time to set up the structure. > > Neither of those operations is speed sensitive, and the more common > > operation > > of picking a specific version can be done in O(n) over an array. So using > > an rbtree will not save any time actually. > > > I was initially using a vector of pointers here for the same reasons you > reiterated, but I felt the use of greedy_realloc0() was messy and > error-prone. greedy_realloc0() is not that messy. And it would be just a few lines of code. We have similar patterns in many other places, and consistency is good.
> The rbtree does not require the use of realloc(). There is no > way to know how long the array needs to be from the start. Even the O(n) > you mention could be turned into O(logn) by using a binary search. Nope. The array needs to be sorted to do a binary search. So the upfront cost of sorting kills any gain you get later on. Zbyszek _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel