[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] (x)memcoll: minor tweaks
From: |
Bruno Haible |
Subject: |
Re: [PATCH] (x)memcoll: minor tweaks |
Date: |
Sun, 11 Jul 2010 14:06:34 +0200 |
User-agent: |
KMail/1.9.9 |
Hi Paul,
There is one API bug and two oddities in your patch:
1) You removed the 'abort' calls in memcoll0 that were intended to verify
that the caller passes meaningful arguments. The documentation that you
added:
/* Compare S1 (with length S1LEN) and S2 (with length S2LEN) according
to the LC_COLLATE locale. S1 and S2 must both end in a null byte.
Set errno to an error number if there is an error, and to zero
otherwise. */
asserts that a valid way to use this function is e.g.
memcoll0 (s1, strlen (s1), s2, strlen (s2)).
(By common understanding, the "length" of a C string is strlen (s).)
But when a user does this, then in strcoll_loop line 46 you
subtract (strlen (s1) + 1) from s1len which is strlen (s1), and
the loop overruns the end of the string and crashes.
I would add back the abort()s as a safeguard. Performance does matter,
but the time spent in a byte lookup and a conditional branch of probability
0 is negligible compared to an strcoll() call which does all kinds of
locale dependent processing.
2) You added documentation to memcoll0 that does not make references to other
functions. Good. Can we have the same for xmemcoll0 as well?
3) There are two 'const char *' left over in memcoll.c lines 33 and 106.
Bruno