pbrowne011
Joined 5 years ago
Joined 5 years ago
"Rule 1. You can't tell where a program is going to spend its time. Bottlenecks occur in surprising places, so don't try to second guess and put in a speed hack until you've proven that's where the bottleneck is.
Rule 2. Measure. Don't tune for speed until you've measured, and even then don't unless one part of the code overwhelms the rest."
Awesome to see someone continuing in the tradition of free/libre OSS: https://linuxclass.heinz.cmu.edu/doc/Open-Source-books/Free-...
X (Twitter): https://x.com/I4Replication/status/1893842119594258913
Bluesky: https://bsky.app/profile/i4replication.bsky.social/post/3liv...
This also goes to show how every line matters. The one line change was from
if (c >= 93 || c>=0xc6-0x81 && d>0x52)
to if (c > 0xc6-0x81 || c==0xc6-0x81 && d>0x52)
Link to relevant source code: https://git.musl-libc.org/cgit/musl/tree/src/locale/iconv.c (bug fix is line 505).To explain the bug, EUC_KR is Extended Unicode - Korean. After ASCII characters (0-127), each character is multiple bytes. The logic adjusts to look up from a base-0 index, then checks that both bytes (variables c and d) are in the normalized range. If not, the code adjusts the characters and checks the bounds again. This second check is what contained the bug.
0xc6 - 0x81 is the upper bound of this check (69), not 93. The irony is that the second part of the check gets this correct. I wonder what a `git blame` would reveal about this particular line.
Also, the developer who wrote the parser (Géza Herman) was able to pass all of the tests with strange edge cases from https://seriot.ch/projects/parsing_json.html very quickly: https://lists.gnu.org/archive/html/emacs-devel/2024-03/msg00...