Patches item #3421072, was opened at 2011-10-10 00:03 Message generated for change (Comment added) made by millerpuckette You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=478072&aid=3421072...
Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: puredata Group: None
Status: Pending Resolution: Accepted
Priority: 5 Private: No Submitted By: Marvin Humphrey (creamygoodness) Assigned to: Miller Puckette (millerpuckette) Summary: Streamline s_utf8.c
Initial Comment: There are a number of UTF-8 manipulation routines which were included when the library was added with the justification that they might be used someday. However, time has passed, they are still unused, and some of them have bugs -- so it is time to delete them with a justification of YAGNI (You Ain't Gonna Need It).
----------------------------------------------------------------------
Comment By: Miller Puckette (millerpuckette)
Date: 2012-12-15 13:04
Message: applied for 0.44
----------------------------------------------------------------------
Comment By: Hans-Christoph Steiner (eighthave) Date: 2011-10-10 13:31
Message: Ok, I say this is ready for Miller to accept, so I'm assigning it to him.
----------------------------------------------------------------------
Comment By: Nobody/Anonymous (nobody) Date: 2011-10-10 13:25
Message: Bryan Jurish:
I didn't see any fix for the NUL-termination problems I saw suggested on the #dataflow transcript though (and I never noticed any such segfaults myself): has that problem been confirmed?
The NUL-termination problems for u8_inc(), u8_charnum() and u8_offset() were fixed under a different issue:
http://sourceforge.net/tracker/?func=detail&atid=478072&aid=3420484&...
Those functions were all capable of overrunning the end of a non-NUL-terminated string; u8_offset() could potentially return an incorrect answer depending on what it found there.
I never duplicated the segfault myself using the steps that Matthieu described in #dataflow; however, it was very easy to detect the invalid reads by running Pd under Valgrind. If I tried hard I might be able to trigger a segfault consistently by carefully crafting bytes to trick u8_offset() into giving too big a number, resulting in a cursor location beyond the end of a string rather than at the end of a string. In any case, the patch both eliminated the invalid reads *and* sped things up, so I think it was justified even without a consistently segfaulting test case.
I suspect that u8_inc_ptr() has problems similar to u8_inc() -- but since it does not seem to be used anywhere at present, I haven't put any time into diagnosing or fixing it. Instead, this patch removes it altogether. We can resurrect it from version control if a need arises in the future and vet it at that time.
The patches attached to this issue should be low-risk, as all they do is remove functionality rather than change behaviors. The removed symbols are unused by Vanilla, and seem also to be unused by Pd-extended. The risk is that I missed some usage somewhere, resulting in link-time failure.
----------------------------------------------------------------------
Comment By: Bryan Jurish (mukau) Date: 2011-10-10 12:16
Message: Looks good to me. I can't test anything right now, but Martin's suggestions seem safe and clear. Give it a whirl...
I didn't see any fix for the NUL-termination problems I saw suggested on the #dataflow transcript though (and I never noticed any such segfaults myself): has that problem been confirmed?
----------------------------------------------------------------------
Comment By: Hans-Christoph Steiner (eighthave) Date: 2011-10-10 09:37
Message: Makes sense to me. I'm assigning it to Bryan Jurish, the original author of the UTF-8 support, to see if he agrees.
----------------------------------------------------------------------
You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=478072&aid=3421072...
-
SourceForge.net