On Tue, 22 Nov 2011, Eitan Adler wrote:
Log:
- add check for pointer equality prior to performing the O(n) pass
This seems like a negative optimization. The pointers are unequal in
the usual case. More seriously, it gives worse undefined behaviour
when the pointers are invalid. E.g., strcmp(NULL, NULL) now returns
0 where it used to trap for the null pointer access.
This has many style bugs.
- while here change 's' to 's1' in strcoll
Submitted by: eadler@
Reviewed by: theraven@
Approved by: brooks@
MFC after: 2 weeks
Modified: head/lib/libc/string/strcasecmp.c
==============================================================================
--- head/lib/libc/string/strcasecmp.c Mon Nov 21 23:32:14 2011
(r227807)
+++ head/lib/libc/string/strcasecmp.c Tue Nov 22 00:07:53 2011
(r227808)
@@ -48,6 +48,9 @@ strcasecmp_l(const char *s1, const char
const u_char
*us1 = (const u_char *)s1,
*us2 = (const u_char *)s2;
+ if (s1 == s2)
+ return (0);
+
This adds non-KNF indentation (3 spaces instead of 1 tab) and an extra
blank line.
Previous locale changes have already mangled this file, with additions
of extra blank lines and worse.
FIX_LOCALE(locale);
while (tolower_l(*us1, locale) == tolower_l(*us2++, locale))
@@ -65,18 +68,21 @@ int
strncasecmp_l(const char *s1, const char *s2, size_t n, locale_t locale)
{
FIX_LOCALE(locale);
- if (n != 0) {
- const u_char
- *us1 = (const u_char *)s1,
- *us2 = (const u_char *)s2;
-
- do {
- if (tolower_l(*us1, locale) != tolower_l(*us2++,
locale))
- return (tolower_l(*us1, locale) -
tolower_l(*--us2, locale));
- if (*us1++ == '\0')
- break;
- } while (--n != 0);
- }
+
+ const u_char
+ *us1 = (const u_char *)s1,
+ *us2 = (const u_char *)s2;
+
This adds a dependency on C99 by placing the declarations after a statement.
Moving the declarations out of an inner block is otherwise good.
+ if (( s1 == s2) | (n == 0))
This adds an extra space, a weird operator (arithmetic OR instead of
logical OR), and extra parentheses. The extra parentheses are more needed
with the weird operator (arithmetic OR has a weird precedence that happens
to make the parentheses unnecessary here, but compilers should warn about
the relative precedences being surprising and force you to add the
parentheses).
+ return (0);
This adds the usual non-KNF indentation.
+
+
This adds 2 extra blank lines instead of only 1.
+ do {
+ if (tolower_l(*us1, locale) != tolower_l(*us2++, locale))
+ return (tolower_l(*us1, locale) - tolower_l(*--us2,
locale));
+ if (*us1++ == '\0')
+ break;
+ } while (--n != 0);
return (0);
}
Modified: head/lib/libc/string/strcmp.c
==============================================================================
--- head/lib/libc/string/strcmp.c Mon Nov 21 23:32:14 2011
(r227807)
+++ head/lib/libc/string/strcmp.c Tue Nov 22 00:07:53 2011
(r227808)
@@ -44,6 +44,9 @@ __FBSDID("$FreeBSD$");
int
strcmp(const char *s1, const char *s2)
{
+ if (s1 == s2)
+ return (0);
+
This adds non-KNF indentation and an extra blank line, and removes the
strange KNF blank line before the (null) declarations.
while (*s1 == *s2++)
if (*s1++ == '\0')
return (0);
Modified: head/lib/libc/string/strcoll.c
OK.
Modified: head/lib/libc/string/strncmp.c
==============================================================================
--- head/lib/libc/string/strncmp.c Mon Nov 21 23:32:14 2011
(r227807)
+++ head/lib/libc/string/strncmp.c Tue Nov 22 00:07:53 2011
(r227808)
@@ -39,8 +39,9 @@ int
strncmp(const char *s1, const char *s2, size_t n)
{
- if (n == 0)
+ if ((n == 0) | (s1 == s2))
return (0);
+
As above, but without removing the blank line before the null declarations,
without the extra space before s1, with only 1 extra blank line, and with
the equality tests reversed relative to the above.
do {
if (*s1 != *s2++)
return (*(const unsigned char *)s1 -
Bruce
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "[email protected]"
