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

[Zbigniew Jędrzejewski-Szmek]

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