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.
This directory contains the -liberty library of free software.
It is a collection of subroutines used by various GNU programs.
Current members include:
getopt -- get options from command line
obstack -- stacks of arbitrarily-sized objects
strerror -- error message strings corresponding to errno
strtol -- string-to-long conversion
strtoul -- string-to-unsigned-long conversion
We expect many of the GNU subroutines that are floating around to
eventually arrive here.
The library must be configured from the top source directory. Don't
try to run configure in this directory. Follow the configuration
instructions in ../README.
Please report bugs to https://gcc.gnu.org/bugzilla/ and send fixes to
"gcc-patches@gcc.gnu.org". Thank you.
ADDING A NEW FILE
=================
There are two sets of files: Those that are "required" will be
included in the library for all configurations, while those
that are "optional" will be included in the library only if "needed."
To add a new required file, edit Makefile.in to add the source file
name to CFILES and the object file to REQUIRED_OFILES.
To add a new optional file, it must provide a single function, and the
name of the function must be the same as the name of the file.
* Add the source file name to CFILES in Makefile.in and the object
file to CONFIGURED_OFILES.
* Add the function to name to the funcs shell variable in
configure.ac.
* Add the function to the AC_CHECK_FUNCS lists just after the
setting of the funcs shell variable. These AC_CHECK_FUNCS calls
are never executed; they are there to make autoheader work
better.
* Consider the special cases of building libiberty; as of this
writing, the special cases are newlib and VxWorks. If a
particular special case provides the function, you do not need
to do anything. If it does not provide the function, add the
object file to LIBOBJS, and add the function name to the case
controlling whether to define HAVE_func.
Finally, in the build directory of libiberty, configure with
"--enable-maintainer-mode", run "make maint-deps" to update
Makefile.in, and run 'make stamp-functions' to regenerate
functions.texi.
The optional file you've added (e.g. getcwd.c) should compile and work
on all hosts where it is needed. It does not have to work or even
compile on hosts where it is not needed.
ADDING A NEW CONFIGURATION
==========================
On most hosts you should be able to use the scheme for automatically
figuring out which files are needed. In that case, you probably
don't need a special Makefile stub for that configuration.
If the fully automatic scheme doesn't work, you may be able to get
by with defining EXTRA_OFILES in your Makefile stub. This is
a list of object file names that should be treated as required
for this configuration - they will be included in libiberty.a,
regardless of whatever might be in the C library.
Jakub Jelinek
1719fa40c4