On Fri, Feb 27, 2015 at 9:03 AM, Zbigniew Jędrzejewski-Szmek < zbys...@in.waw.pl> wrote: > > We need two operations: sorting kernels to list them, and picking (I
> 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. > > > > > > 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. > Oh I see, you don't have to sort in that case. But the code would be longer, so perhaps I will sort in all cases. > > Zbyszek > _______________________________________________ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel > -- Shawn Landden
_______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel