[systemd-devel] [PATCH 2/2] test: test for ellipsize (manual)

[Shawn]

On Sun, Oct 13, 2013 at 4:46 PM, Zbigniew Jędrzejewski-Szmek <
zbyszek at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20131017/ab32ffe9/attachment-0001.html>