Patchwork add strnlen to libiberty (was Re: Assembly output optimisations)

login
register
mail settings
Submitter Dimitrios Apostolou
Date Aug. 7, 2012, 4:34 a.m.
Message ID <alpine.LNX.2.02.1208070728170.20463@localhost.localdomain>
Download mbox | patch
Permalink /patch/175501/
State New
Headers show

Comments

Dimitrios Apostolou - Aug. 7, 2012, 4:34 a.m.
As an addendum to my previous patch, I made an attempt to properly add
strnlen() to libiberty, with the code copied from gnulib. Unfortunately it 
seems I've messed it up somewhere since defining HAVE_STRNLEN to 0 doesn't 
seem to build strnlen.o for me. Any ideas?


Thanks,
Dimitris

=== modified file 'gcc/config.in'
--- gcc/config.in	2012-05-25 09:24:08 +0000
+++ gcc/config.in	2012-08-07 01:15:45 +0000
@@ -828,6 +828,13 @@
 #endif
 
 
+/* Define to 1 if we found a declaration for 'strnlen', otherwise define to 0.
+   */
+#ifndef USED_FOR_TARGET
+#undef HAVE_DECL_STRNLEN
+#endif
+
+
 /* Define to 1 if we found a declaration for 'strsignal', otherwise define to
    0. */
 #ifndef USED_FOR_TARGET

=== modified file 'gcc/configure'
--- gcc/configure	2012-07-24 09:49:56 +0000
+++ gcc/configure	2012-08-07 01:02:28 +0000
@@ -10462,7 +10462,7 @@ CFLAGS="$CFLAGS -I${srcdir} -I${srcdir}/
 saved_CXXFLAGS="$CXXFLAGS"
 CXXFLAGS="$CXXFLAGS -I${srcdir} -I${srcdir}/../include"
 for ac_func in getenv atol asprintf sbrk abort atof getcwd getwd \
-	strsignal strstr stpcpy strverscmp \
+	strsignal strstr stpcpy strverscmp strnlen \
 	errno snprintf vsnprintf vasprintf malloc realloc calloc \
 	free basename getopt clock getpagesize clearerr_unlocked feof_unlocked   ferror_unlocked fflush_unlocked fgetc_unlocked fgets_unlocked   fileno_unlocked fprintf_unlocked fputc_unlocked fputs_unlocked   fread_unlocked fwrite_unlocked getchar_unlocked getc_unlocked   putchar_unlocked putc_unlocked
 do
@@ -17973,7 +17973,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 17975 "configure"
+#line 17976 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -18079,7 +18079,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 18081 "configure"
+#line 18082 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H

=== modified file 'gcc/configure.ac'
--- gcc/configure.ac	2012-07-24 09:49:56 +0000
+++ gcc/configure.ac	2012-08-07 01:38:59 +0000
@@ -1087,7 +1087,7 @@ CFLAGS="$CFLAGS -I${srcdir} -I${srcdir}/
 saved_CXXFLAGS="$CXXFLAGS"
 CXXFLAGS="$CXXFLAGS -I${srcdir} -I${srcdir}/../include"
 gcc_AC_CHECK_DECLS(getenv atol asprintf sbrk abort atof getcwd getwd \
-	strsignal strstr stpcpy strverscmp \
+	strsignal strstr stpcpy strverscmp strnlen \
 	errno snprintf vsnprintf vasprintf malloc realloc calloc \
 	free basename getopt clock getpagesize gcc_UNLOCKED_FUNCS, , ,[
 #include "ansidecl.h"

=== modified file 'gcc/system.h'
--- gcc/system.h	2012-07-24 22:25:18 +0000
+++ gcc/system.h	2012-08-07 01:54:49 +0000
@@ -451,6 +451,10 @@ extern char *strstr (const char *, const
 extern char *stpcpy (char *, const char *);
 #endif
 
+#if defined (HAVE_DECL_STRNLEN) && !HAVE_DECL_STRNLEN
+extern size_t strnlen(const char *, size_t);
+#endif
+
 #ifdef __cplusplus
 }
 #endif
Ian Taylor - Aug. 7, 2012, 4:56 a.m.
On Mon, Aug 6, 2012 at 9:34 PM, Dimitrios Apostolou <jimis@gmx.net> wrote:
> As an addendum to my previous patch, I made an attempt to properly add
> strnlen() to libiberty, with the code copied from gnulib. Unfortunately it
> seems I've messed it up somewhere since defining HAVE_STRNLEN to 0 doesn't
> seem to build strnlen.o for me. Any ideas?

What do you mean by "defining HAVE_STRNLEN to 0"?  The thing that will
control building strnlen.o is having AC_REPLACE_FUNCS in
libiberty/configure.in fail to find strlen.  One way you can test this
is by adding this to libiberty/config.cache:

ac_cv_func_strnlen=${ac_cv_func_strnlen=yes}

before invoking configure.

Ian
Dimitrios Apostolou - Aug. 7, 2012, 5:44 a.m.
On Mon, 6 Aug 2012, Ian Lance Taylor wrote:

> On Mon, Aug 6, 2012 at 9:34 PM, Dimitrios Apostolou <jimis@gmx.net> wrote:
>> As an addendum to my previous patch, I made an attempt to properly add
>> strnlen() to libiberty, with the code copied from gnulib. Unfortunately it
>> seems I've messed it up somewhere since defining HAVE_STRNLEN to 0 doesn't
>> seem to build strnlen.o for me. Any ideas?
>
> What do you mean by "defining HAVE_STRNLEN to 0"?  The thing that will
> control building strnlen.o is having AC_REPLACE_FUNCS in
> libiberty/configure.in fail to find strlen.  One way you can test this
> is by adding this to libiberty/config.cache:
>
> ac_cv_func_strnlen=${ac_cv_func_strnlen=yes}
>
> before invoking configure.

Thanks Ian, that helped a lot. I changed ac_cv_func_strnlen=no and 
reconfigured with -C, and strnlen.o was built. :-)

What else is missing to make this patch appropriate for libiberty? Should 
I change the prolog in strnlen.c, since I only copied it intact from 
gnulib?


Thanks,
Dimitris
Ian Taylor - Aug. 7, 2012, 6:25 a.m.
On Mon, Aug 6, 2012 at 10:44 PM, Dimitrios Apostolou <jimis@gmx.net> wrote:
>
> What else is missing to make this patch appropriate for libiberty? Should I
> change the prolog in strnlen.c, since I only copied it intact from gnulib?

We generally try to avoid straight GPL source code without runtime
exception in libiberty, and I don't see a reason to bend that rule for
this trivial function.  I recommend that you write your own version
rather than copying it from elsewhere.

Ian
Hans-Peter Nilsson - Aug. 7, 2012, 9:30 a.m.
On Mon, 6 Aug 2012, Ian Lance Taylor wrote:
> On Mon, Aug 6, 2012 at 10:44 PM, Dimitrios Apostolou <jimis@gmx.net> wrote:
> >
> > What else is missing to make this patch appropriate for libiberty? Should I
> > change the prolog in strnlen.c, since I only copied it intact from gnulib?
>
> We generally try to avoid straight GPL source code without runtime
> exception in libiberty, and I don't see a reason to bend that rule for
> this trivial function.

Just don't forget that libiberty isn't a target library anymore.
To wit, the (GCC) run-time exception is moot for that code, AFAIK.
Maybe enough reason to abandon that rule so its code can be
truly and freely shared between GNU projects.

brgds, H-P
Ian Taylor - Aug. 7, 2012, 1:44 p.m.
On Tue, Aug 7, 2012 at 2:30 AM, Hans-Peter Nilsson <hp@bitrange.com> wrote:
>
> Just don't forget that libiberty isn't a target library anymore.
> To wit, the (GCC) run-time exception is moot for that code, AFAIK.
> Maybe enough reason to abandon that rule so its code can be
> truly and freely shared between GNU projects.

The libiberty licensing is certainly confused.  I just don't want to
make it worse.

None of the code in libiberty is under the GCC Runtime Library
Exception, so I think that particular issue does not apply.

Ian
Joseph S. Myers - Aug. 7, 2012, 3:07 p.m.
On Tue, 7 Aug 2012, Ian Lance Taylor wrote:

> On Tue, Aug 7, 2012 at 2:30 AM, Hans-Peter Nilsson <hp@bitrange.com> wrote:
> >
> > Just don't forget that libiberty isn't a target library anymore.
> > To wit, the (GCC) run-time exception is moot for that code, AFAIK.
> > Maybe enough reason to abandon that rule so its code can be
> > truly and freely shared between GNU projects.
> 
> The libiberty licensing is certainly confused.  I just don't want to
> make it worse.

I think the natural way to sort it out is to move all the FSF-copyright 
files therein (including include/) to GPLv3, no license exception, except 
for cp-demangle.c (used in libstdc++-v3) and the headers it includes, 
which should have the GCC Runtime Library Exception notice.  libiberty is 
a library for a limited number of GNU projects, all under GPLv3; as far as 
I know the only reason it hasn't been converted to GPLv3 is that noone got 
around to doing so.  (gnulib also uses the practice of putting GPLv3 
license notices on the files even if they are also available under other 
licenses, with separate metadata indicating other licenses under which 
files are available.)

That wouldn't sort out the question of what "This file is part of" notices 
should be present, but would deal with the other license confusion.

(Ideally I think most of libiberty would be replaced by use of gnulib in 
the projects using libiberty - I see no real advantage to the GNU Project 
in having those two separate libraries for substantially the same purposes 
- but that's a much larger and harder task, which would also involve, for 
each libiberty file replaced by a gnulib version, ascertaining whether 
there are any features or local changes in the libiberty version that 
should be merged into the gnulib version or any common upstream such as 
glibc.  And some files in libiberty would probably need adding to gnulib 
as part of such a project.)

Patch

=== modified file 'libiberty/Makefile.in'
--- libiberty/Makefile.in	2012-04-27 14:14:14 +0000

+++ libiberty/Makefile.in	2012-08-07 03:52:53 +0000

@@ -151,7 +151,7 @@  CFILES = alloca.c argv.c asprintf.c atex

 	 spaces.c splay-tree.c stack-limit.c stpcpy.c stpncpy.c		\
 	 strcasecmp.c strchr.c strdup.c strerror.c strncasecmp.c	\
 	 strncmp.c strrchr.c strsignal.c strstr.c strtod.c strtol.c	\
-	 strtoul.c strndup.c strverscmp.c				\

+	 strtoul.c strndup.c strnlen.c strverscmp.c			\

 	timeval-utils.c tmpnam.c					\
 	unlink-if-ordinary.c						\
 	vasprintf.c vfork.c vfprintf.c vprintf.c vsnprintf.c vsprintf.c	\
@@ -218,6 +218,7 @@  CONFIGURED_OFILES = ./asprintf.$(objext)

 	 ./strncmp.$(objext) ./strndup.$(objext) ./strrchr.$(objext)	\
 	 ./strstr.$(objext) ./strtod.$(objext) ./strtol.$(objext)	\
 	 ./strtoul.$(objext) ./strverscmp.$(objext)			\
+	./strnlen.$(objext)						\

 	./tmpnam.$(objext)						\
 	./vasprintf.$(objext) ./vfork.$(objext) ./vfprintf.$(objext)	\
 	 ./vprintf.$(objext) ./vsnprintf.$(objext) ./vsprintf.$(objext)	\
@@ -622,8 +623,7 @@  $(CONFIGURED_OFILES): stamp-picdir

 	else true; fi
 	$(COMPILE.c) $(srcdir)/crc32.c $(OUTPUT_OPTION)
 
-./dwarfnames.$(objext): $(srcdir)/dwarfnames.c $(INCDIR)/dwarf2.h \

-	$(INCDIR)/dwarf2.def

+./dwarfnames.$(objext): $(srcdir)/dwarfnames.c $(INCDIR)/dwarf2.h

 	if [ x"$(PICFLAG)" != x ]; then \
 	  $(COMPILE.c) $(PICFLAG) $(srcdir)/dwarfnames.c -o pic/$@; \
 	else true; fi
@@ -656,7 +656,8 @@  $(CONFIGURED_OFILES): stamp-picdir

 	else true; fi
 	$(COMPILE.c) $(srcdir)/fibheap.c $(OUTPUT_OPTION)
 
-./filename_cmp.$(objext): $(srcdir)/filename_cmp.c config.h $(INCDIR)/filenames.h \

+./filename_cmp.$(objext): $(srcdir)/filename_cmp.c config.h $(INCDIR)/ansidecl.h \

+	$(INCDIR)/filenames.h $(INCDIR)/hashtab.h \

 	$(INCDIR)/safe-ctype.h
 	if [ x"$(PICFLAG)" != x ]; then \
 	  $(COMPILE.c) $(PICFLAG) $(srcdir)/filename_cmp.c -o pic/$@; \
@@ -757,7 +758,7 @@  $(CONFIGURED_OFILES): stamp-picdir

 	$(COMPILE.c) $(srcdir)/insque.c $(OUTPUT_OPTION)
 
 ./lbasename.$(objext): $(srcdir)/lbasename.c config.h $(INCDIR)/ansidecl.h \
-	$(INCDIR)/filenames.h $(INCDIR)/libiberty.h \

+	$(INCDIR)/filenames.h $(INCDIR)/hashtab.h $(INCDIR)/libiberty.h \

 	$(INCDIR)/safe-ctype.h
 	if [ x"$(PICFLAG)" != x ]; then \
 	  $(COMPILE.c) $(PICFLAG) $(srcdir)/lbasename.c -o pic/$@; \
@@ -1043,7 +1044,7 @@  $(CONFIGURED_OFILES): stamp-picdir

 	else true; fi
 	$(COMPILE.c) $(srcdir)/splay-tree.c $(OUTPUT_OPTION)
 
-./stack-limit.$(objext): $(srcdir)/stack-limit.c config.h

+./stack-limit.$(objext): $(srcdir)/stack-limit.c config.h $(INCDIR)/ansidecl.h

 	if [ x"$(PICFLAG)" != x ]; then \
 	  $(COMPILE.c) $(PICFLAG) $(srcdir)/stack-limit.c -o pic/$@; \
 	else true; fi
@@ -1104,6 +1105,12 @@  $(CONFIGURED_OFILES): stamp-picdir

 	else true; fi
 	$(COMPILE.c) $(srcdir)/strndup.c $(OUTPUT_OPTION)
 
+./strnlen.$(objext): $(srcdir)/strnlen.c

+	if [ x"$(PICFLAG)" != x ]; then \

+	  $(COMPILE.c) $(PICFLAG) $(srcdir)/strnlen.c -o pic/$@; \

+	else true; fi

+	$(COMPILE.c) $(srcdir)/strnlen.c $(OUTPUT_OPTION)

+

 ./strrchr.$(objext): $(srcdir)/strrchr.c $(INCDIR)/ansidecl.h
 	if [ x"$(PICFLAG)" != x ]; then \
 	  $(COMPILE.c) $(PICFLAG) $(srcdir)/strrchr.c -o pic/$@; \

=== modified file 'libiberty/config.in'
--- libiberty/config.in	2011-07-22 08:33:37 +0000

+++ libiberty/config.in	2012-08-07 03:29:18 +0000

@@ -262,6 +262,9 @@ 

 /* Define to 1 if you have the `strndup' function. */
 #undef HAVE_STRNDUP
 
+/* Define to 1 if you have the `strnlen' function. */

+#undef HAVE_STRNLEN

+

 /* Define to 1 if you have the `strrchr' function. */
 #undef HAVE_STRRCHR
 

=== modified file 'libiberty/configure'
--- libiberty/configure	2012-01-23 06:25:28 +0000

+++ libiberty/configure	2012-08-07 03:10:54 +0000

@@ -5340,6 +5340,7 @@  funcs="$funcs strchr"

 funcs="$funcs strdup"
 funcs="$funcs strncasecmp"
 funcs="$funcs strndup"
+funcs="$funcs strnlen"

 funcs="$funcs strrchr"
 funcs="$funcs strstr"
 funcs="$funcs strtod"
@@ -5380,8 +5381,8 @@  if test "x" = "y"; then

     random realpath rename rindex \
     sbrk setenv setproctitle setrlimit sigsetmask snprintf spawnve spawnvpe \
      stpcpy stpncpy strcasecmp strchr strdup \
-     strerror strncasecmp strndup strrchr strsignal strstr strtod strtol \

-     strtoul strverscmp sysconf sysctl sysmp \

+     strerror strncasecmp strndup strnlen strrchr strsignal strstr strtod \

+     strtol strtoul strverscmp sysconf sysctl sysmp \

     table times tmpnam \
     vasprintf vfprintf vprintf vsprintf \
     wait3 wait4 waitpid

=== modified file 'libiberty/configure.ac'
--- libiberty/configure.ac	2011-08-22 16:54:02 +0000

+++ libiberty/configure.ac	2012-08-07 03:10:42 +0000

@@ -322,6 +322,7 @@  funcs="$funcs strchr"

 funcs="$funcs strdup"
 funcs="$funcs strncasecmp"
 funcs="$funcs strndup"
+funcs="$funcs strnlen"

 funcs="$funcs strrchr"
 funcs="$funcs strstr"
 funcs="$funcs strtod"
@@ -362,8 +363,8 @@  if test "x" = "y"; then

     random realpath rename rindex \
     sbrk setenv setproctitle setrlimit sigsetmask snprintf spawnve spawnvpe \
      stpcpy stpncpy strcasecmp strchr strdup \
-     strerror strncasecmp strndup strrchr strsignal strstr strtod strtol \

-     strtoul strverscmp sysconf sysctl sysmp \

+     strerror strncasecmp strndup strnlen strrchr strsignal strstr strtod \

+     strtol strtoul strverscmp sysconf sysctl sysmp \

     table times tmpnam \
     vasprintf vfprintf vprintf vsprintf \
     wait3 wait4 waitpid)

=== modified file 'libiberty/functions.texi'
--- libiberty/functions.texi	2011-02-28 18:23:25 +0000

+++ libiberty/functions.texi	2012-08-07 03:46:29 +0000

@@ -1574,6 +1574,14 @@  memory was available.  The result is alw

 
 @end deftypefn
 
+@c strnlen.c:20

+@deftypefn Extension size_t strnlen (const char *@var{string}, size_t @var{maxlen})

+

+Find the length of @var{string}, but scan at most @var{maxlen} characters.  If

+no '\0' terminator is found in that many characters, return @var{maxlen}.

+

+@end deftypefn

+

 @c strrchr.c:6
 @deftypefn Supplemental char* strrchr (const char *@var{s}, int @var{c})
 

=== added file 'libiberty/strnlen.c'
--- libiberty/strnlen.c	1970-01-01 00:00:00 +0000

+++ libiberty/strnlen.c	2012-08-07 03:54:45 +0000

@@ -0,0 +1,37 @@ 

+/* Find the length of STRING, but scan at most MAXLEN characters.

+   Copyright (C) 2005-2007, 2009-2012 Free Software Foundation, Inc.

+   Written by Simon Josefsson.

+

+   This program is free software; you can redistribute it and/or modify

+   it under the terms of the GNU General Public License as published by

+   the Free Software Foundation; either version 2, or (at your option)

+   any later version.

+

+   This program is distributed in the hope that it will be useful,

+   but WITHOUT ANY WARRANTY; without even the implied warranty of

+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

+   GNU General Public License for more details.

+

+   You should have received a copy of the GNU General Public License

+   along with this program; if not, see <http://www.gnu.org/licenses/>.  */

+

+/*

+

+@deftypefn Extension size_t strnlen (const char *@var{string}, size_t @var{maxlen})

+

+Find the length of @var{string}, but scan at most @var{maxlen} characters.  If

+no '\0' terminator is found in that many characters, return @var{maxlen}.

+

+@end deftypefn

+

+*/

+

+#include <string.h>

+#include <stddef.h>

+

+size_t

+strnlen (const char *string, size_t maxlen)

+{

+  const char *end = (const char *) memchr (string, '\0', maxlen);

+  return end ? (size_t) (end - string) : maxlen;

+}