public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r11-10733] libiberty: Make strstr.c in libiberty ANSI compliant
@ 2023-05-02 20:16 Jakub Jelinek
  0 siblings, 0 replies; only message in thread
From: Jakub Jelinek @ 2023-05-02 20:16 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:83fe692a373f6e6738f4bdc6661d1d58e4bf95c6

commit r11-10733-g83fe692a373f6e6738f4bdc6661d1d58e4bf95c6
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sun Apr 2 20:05:31 2023 +0200

    libiberty: Make strstr.c in libiberty ANSI compliant
    
    On Fri, Nov 13, 2020 at 11:53:43AM -0700, Jeff Law via Gcc-patches wrote:
    >
    > On 5/1/20 6:06 PM, Seija Kijin via Gcc-patches wrote:
    > > The original code in libiberty says "FIXME" and then says it has not been
    > > validated to be ANSI compliant. However, this patch changes the function to
    > > match implementations that ARE compliant, and such code is in the public
    > > domain.
    > >
    > > I ran the test results, and there are no test failures.
    >
    > Thanks.  This seems to be the standard "simple" strstr implementation. 
    > There's significantly faster implementations available, but I doubt it's
    > worth the effort as the version in this file only gets used if there is
    > no system strstr.c.
    
    Except that PR109306 says the new version is non-compliant and
    is certainly slower than what we used to have.  The only problem I see
    on the old version (sure, it is not very fast version) is that for
    strstr ("abcd", "") it returned "abcd"+4 rather than "abcd" because
    strchr in that case changed p to point to the last character and then
    strncmp returned 0.
    
    The question reported in PR109306 is whether memcmp is required not to
    access characters beyond the first difference or not.
    For all of memcmp/strcmp/strncmp, C17 says:
    "The sign of a nonzero value returned by the comparison functions memcmp, strcmp, and strncmp
    is determined by the sign of the difference between the values of the first pair of characters (both
    interpreted as unsigned char) that differ in the objects being compared."
    but then in memcmp description says:
    "The memcmp function compares the first n characters of the object pointed to by s1 to the first n
    characters of the object pointed to by s2."
    rather than something similar to strncmp wording:
    "The strncmp function compares not more than n characters (characters that follow a null character
    are not compared) from the array pointed to by s1 to the array pointed to by
    s2."
    So, while for strncmp it seems clearly well defined when there is zero
    terminator before reaching the n, for memcmp it is unclear if say
    int
    memcmp (const void *s1, const void *s2, size_t n)
    {
      int ret = 0;
      size_t i;
      const unsigned char *p1 = (const unsigned char *) s1;
      const unsigned char *p2 = (const unsigned char *) s2;
    
      for (i = n; i; i--)
        if (p1[i - 1] != p2[i - 1])
          ret = p1[i - 1] < p2[i - 1] ? -1 : 1;
      return ret;
    }
    wouldn't be valid implementation (one which always compares all characters
    and just returns non-zero from the first one that differs).
    
    So, shouldn't we just revert and handle the len == 0 case correctly?
    
    I think almost nothing really uses it, but still, the old version
    at least worked nicer with a fast strchr.
    Could as well strncmp (p + 1, s2 + 1, len - 1) if that is preferred
    because strchr already compared the first character.
    
    2023-04-02  Jakub Jelinek  <jakub@redhat.com>
    
            PR other/109306
            * strstr.c: Revert the 2020-11-13 changes.
            (strstr): Return s1 if len is 0.
    
    (cherry picked from commit 1719fa40c4ee4def60a2ce2f27e17f8168cf28ba)

Diff:
---
 libiberty/strstr.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/libiberty/strstr.c b/libiberty/strstr.c
index c6f68495a33..49209e8229f 100644
--- a/libiberty/strstr.c
+++ b/libiberty/strstr.c
@@ -18,18 +18,23 @@ length, the function returns @var{string}.
 
 #include <stddef.h>
 
-extern int memcmp (const void *, const void *, size_t);
+extern char *strchr (const char *, int);
+extern int strncmp (const void *, const void *, size_t);
 extern size_t strlen (const char *);
 
 char *
 strstr (const char *s1, const char *s2)
 {
+  const char *p = s1;
   const size_t len = strlen (s2);
-  while (*s1)
+
+  if (!len)
+    return s1;
+
+  for (; (p = strchr (p, *s2)) != 0; p++)
     {
-      if (!memcmp (s1, s2, len))
-	return (char *)s1;
-      ++s1;
+      if (strncmp (p, s2, len) == 0)
+	return (char *)p;
     }
   return (0);
 }

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-05-02 20:16 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-02 20:16 [gcc r11-10733] libiberty: Make strstr.c in libiberty ANSI compliant Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).