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

Zbigniew Jędrzejewski-Szmek zbyszek at in.waw.pl
Sat Oct 5 20:20:58 PDT 2013


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?

Zbyszek


More information about the systemd-devel mailing list