On Sun, Oct 13, 2013 at 4:46 PM, Zbigniew Jędrzejewski-Szmek < zbys...@in.waw.pl> wrote:
> 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. > Thanks for your work on this and sorry for not getting around to reviewing your work. > > Zbyszek > _______________________________________________ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel >
_______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel