On Sun, Oct 06, 2013 at 05:20:58AM +0200, Zbigniew Jędrzejewski-Szmek wrote: > Hi, > > sorry for the long delay. I was expecting this review to take > some time, and I was right :) > > I started by running test-ellipsize under valgrind, and found > that there unitialized memory was written. There was a bug in > ellipsize_mem: when the string was short enough to fit whole, > the middle part was used twice. E.g. abcdef was ellipsized as > abcd...cdef. But space was allocated without this overlap, so > valgrind was unhappy. > > Also, there was some overlap with existing functions, so I managed > to remove a large part of the patch. > > I moved the new code to a separate file, to avoid mixing code > written in two copletely different styles in the same file. I hope > it'll make it easier to pull changes from glib in the future. > > I think that ellipsize_ascii should be removed, small saving > in allocated memory probably isn't worth the extra scan which > is done to check if the string is pure ASCII. Also, it's not nice > that ellipsization is different for pure ASCII and ASCII strings. > I think we should use the same character(s) in both cases. > But I left it as is for now. > > I also optimized the function a bit, so that memory is not zeroed > after allocation, and unicode verification is performed while > counting the characters. > > I found that the COW FACE sequence is not ellipsized properly, because > COW FACEs are fullwidth. I've added this whole block to > the fullwidth table in a separate patch. > > All in all, those are pretty significant changes. Therefore, > instead of pushing, I've posted the changes at > http://in.waw.pl/git/systemd/ > Could you have a look at the updated patches, and maybe test them > a bit? Pushed now.
Zbyszek _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
