On Thu, Jul 18, 2013 at 01:05:25PM +0200, Jan Janssen wrote: > On 07/18/2013 06:10 AM, Zbigniew Jędrzejewski-Szmek wrote: > >On Tue, Jul 16, 2013 at 05:46:04PM +0200, Lennart Poettering wrote: > >>On Tue, 16.07.13 17:42, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) > >>wrote: > >> > >>> > >>>On Tue, Jul 16, 2013 at 05:39:54PM +0200, Lennart Poettering wrote: > >>>>On Fri, 28.06.13 17:26, Jan Janssen (medhe...@web.de) wrote: > >>>>Applied this one now. If people start complaining about its speed we can > >>>>reinvestigate and do find some way for optimization... > >>>We need to think about negative matches. Looking for previous boots > >>>with negative matches should work nicely. > >> > >>The bisection tables make this less efficient but certainly possible. > >> > >>>I'd like to complain about the : in the syntax though. > >> > >>Hmm, what would you propose? There's still time to fix it! > >I went ahead, and removed : from the syntax. It feels OK in my testing. > > > >And I also made one optimization, which is important imho: 'journactl -b' > >will still use the boot id from sd_id128_get_boot() to avoid searching > >through the tables, and 'journalctl -b BOOT_ID[+-0]' will just > >use BOOT_ID without searching through the tables. This should help > >a lot when running with cold cache. > > > >Zbyszek > > > > I really don't like arguments to options that can start with "-", it > can easily be confused with another option. Especially if one were ever > to introduce options like "-0" to "-9". Also, not accepting long UUIDs > is kind of restricting the user. But ultimately, this is > bike-shedding... > > But more importantly, you've introduced a bug: > > $ ./journalctl -b a709bdcbaa1b422f8338a25fd2d4d61d > Relative boot ID offset must start with a '+' or a '-', found '' Arrgh.
> Also, for the challenged people (me), does this really guard the > array access (count >= INT_MAX comes to my mind)? And if so, how? > > if (relative > (int) count || relative <= -(int)count) count comes from searching throught the database, so cannot really get too large without a really long wait. Nevertheless, if it was >= INT_MAX, than after casting to (int) it would become negative, and the left side of the if should be satisfied. > If you could silence this warning, it would be nice :P > > src/journal/journalctl.c: In function ‘get_relative_boot_id’: > src/journal/journalctl.c:747:63: warning: comparison between signed > and unsigned integer expressions [-Wsign-compare] > (id - all_ids) + relative >= count) Interesting that my gcc (4.7.2) didn't show that. > Anyway, gonna go sulk now for not having come up with such nice code > in the first place :( The pull towards bikeshedding and µ-opts was too strong for me to resist :) But I seriously think that the arguments with a dash will turn out OK in practice. Zbyszek -- they are not broken. they are refucktored -- alxchk _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel