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