I added a slightly improved useless-if-before-free to gnulib, so this patch makes bootstrap pull the script from there. It also pulls in a vasnprintf module improvement: it works around a just-discovered bug in HPUX 10.20's printf(3) implementation: http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/12505/focus=12547 Here's the patch: Pull useless-if-before-free from gnulib, and update. * bootstrap (gnulib_tool): Add useless-if-before-free to the list of modules. * build-aux/useless-if-before-free: Update from gnulib. * gnulib/lib/vasnprintf.c: Work around a bug in HPUX 10.20. * gnulib/m4/vasnprintf.m4: Likewise. --- bootstrap | 1 + build-aux/useless-if-before-free | 96 +++++++++++++++++++++++++++++-------- gnulib/lib/Makefile.am | 9 +++- gnulib/lib/vasnprintf.c | 37 ++++++++++----- gnulib/m4/gnulib-cache.m4 | 4 +- gnulib/m4/gnulib-comp.m4 | 1 + gnulib/m4/vasnprintf.m4 | 20 +++++++- 7 files changed, 130 insertions(+), 38 deletions(-) diff --git a/bootstrap b/bootstrap index 1627086..d8b79d9 100755 --- a/bootstrap +++ b/bootstrap @@ -78,6 +78,7 @@ $gnulib_tool \ --import physmem getaddrinfo \ sys_stat vasprintf strndup \ strsep poll gettext getpass \ + useless-if-before-free \ vc-list-files rm -f \ diff --git a/build-aux/useless-if-before-free b/build-aux/useless-if-before-free index 57040a3..eb18483 100755 --- a/build-aux/useless-if-before-free +++ b/build-aux/useless-if-before-free @@ -1,24 +1,43 @@ #!/usr/bin/perl -T # Detect instances of "if (p) free (p);". -# Likewise for "if (p != NULL) free (p);". +# Likewise for "if (p != NULL) free (p);". And with braces. -my $VERSION = '2008-02-04 22:25'; # UTC +my $VERSION = '2008-02-11 08:08'; # UTC # The definition above must lie within the first 8 lines in order # for the Emacs time-stamp write hook (at end) to update it. # If you change this file with Emacs, please let the write hook # do its job. Otherwise, update this string manually. -# Exit status is like grep: 0 for no match. 1 for any number. -# Note: giving line numbers isn't practical, since I've reset the -# input record separator. +# Copyright (C) 2008 Free Software Foundation, Inc. + +# 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 3 of the License, 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/>;. + +# Written by Jim Meyering + use strict; use warnings; use Getopt::Long; (my $ME = $0) =~ s|.*/||; -my $debug = 0; -my $verbose = 0; +# use File::Coda; # http://meyering.net/code/Coda/ +END { + defined fileno STDOUT or return; + close STDOUT and return; + warn "$ME: failed to close standard output: $!\n"; + $? ||= 1; +} sub usage ($) { @@ -33,18 +52,32 @@ sub usage ($) print $STREAM <<EOF; Usage: $ME [OPTIONS] FILE... +Detect any instance in FILE of a useless "if" test before a free call, e.g., +"if (p) free (p);". Any such test may be safely removed without affecting +the semantics of the C code in FILE. Use --name=FOO --name=BAR to also +detect free-like functions named FOO and BAR. + OPTIONS: + --list print only the name of each matching FILE (\0-terminated) --name=N add name N to the list of `free'-like functions to detect; may be repeated --help display this help and exit --version output version information and exit - --verbose generate verbose output + +Exit status: + + 0 no match + 1 one or more matches + 2 an error EXAMPLE: - git ls-files -z |xargs -0 $ME +For example, this command prints all removable "if" tests before "free" +and "kfree" calls in the linux kernel sources: + + git ls-files -z |xargs -0 $ME --name=kfree EOF } @@ -52,32 +85,40 @@ EOF } { + sub EXIT_MATCH {0} + sub EXIT_NO_MATCH {1} + sub EXIT_ERROR {2} + my $err = EXIT_NO_MATCH; + + my $list; my @name = qw(free); GetOptions ( - debug => \$debug, - verbose => \$verbose, help => sub { usage 0 }, version => sub { print "$ME version $VERSION\n"; exit }, + list => \$list, 'name=s@' => \@name, ) or usage 1; # Make sure we have the right number of non-option arguments. # Always tell the user why we fail. @ARGV < 1 - and (warn "$ME: missing FILE argument\n"), usage 1; + and (warn "$ME: missing FILE argument\n"), usage EXIT_ERROR; my $or = join '|', @name; my $regexp = qr/(?:$or)/; # Set the input record separator. + # Note: this makes it impractical to print line numbers. $/ = '"'; my $found_match = 0; + FILE: foreach my $file (@ARGV) { open FH, '<', $file - or die "$ME: can't open `$file' for reading: $!\n"; + or (warn "$ME: can't open `$file' for reading: $!\n"), + $err = EXIT_ERROR, next; while (defined (my $line = <FH>)) { if ($line =~ @@ -85,27 +126,40 @@ EOF (?: \s*$regexp\s*\(\s*\2\s*\)| \s*\{\s*$regexp\s*\(\s*\2\s*\)\s*;\s*\}))/sx) { - print "$file: $1\n"; $found_match = 1; + $list + and (print "$file\0"), next FILE; + print "$file: $1\n"; } } + } + continue + { close FH; } - exit !$found_match; + + $found_match && $err == EXIT_NO_MATCH + and $err = EXIT_MATCH; + + exit $err; } my $foo = <<'EOF'; # The above is to *find* them. # This adjusts them, removing the unnecessary "if (p)" part. -# FIXME: do something like this as an option. -git ls-files -z --exclude=$ME |xargs -0 \ -perl -0x3b -pi -e 's/\bif\s*\(\s*(\S+?)(?:\s*!=\s*NULL)?\s*\)\s+((?:sexpr_)?free\s*\(\s*\1\s*\))/$2/s' +# FIXME: do something like this as an option (doesn't do braces): +git ls-files -z |xargs -0 \ +perl -0x3b -pi -e 's/\bif\s*\(\s*(\S+?)(?:\s*!=\s*NULL)?\s*\)\s+(k?free\s*\(\s*\1\s*\))/$2/s' -When modifying files, refuse to process anything other than a regular file. +useless-if-before-free -l $(lid -knone free) | xargs -0 \ + perl -0x3b -pi -e \ + 's/\bif\s*\(\s*(\S+?)(?:\s*!=\s*NULL)?\s*\)\s+(free\s*\(\s*\1\s*\))/$2/s' -# Or this one-liner to detect them: -git ls-files -z |xargs -0 perl -00 -ne '/\b(if\s*\(\s*(\S+?)(?:\s*!=\s*NULL)?\s*\)(?:\s*(?:sexpr_)?free\s*\(\s*\2\s*\)|\s*\{\s*(?:sexpr_)?free\s*\(\s*\2\s*\)\s*;\s*\}))/sx and print "$1\n"' +Be careful that the result of the above transformation is valid. +If the matched string is followed by "else", then obviously, it won't be. + +When modifying files, refuse to process anything other than a regular file. EOF ## Local Variables: diff --git a/gnulib/lib/Makefile.am b/gnulib/lib/Makefile.am index 8f545ca..9495caf 100644 --- a/gnulib/lib/Makefile.am +++ b/gnulib/lib/Makefile.am @@ -9,7 +9,7 @@ # the same distribution terms as the rest of that program. # # Generated by gnulib-tool. -# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=gnulib/lib --m4-base=gnulib/m4 --doc-base=doc --aux-dir=build-aux --with-tests --lgpl=2 --libtool --macro-prefix=gl getaddrinfo getpass gettext physmem poll strndup strsep sys_stat vasprintf vc-list-files +# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=gnulib/lib --m4-base=gnulib/m4 --doc-base=doc --aux-dir=build-aux --with-tests --lgpl=2 --libtool --macro-prefix=gl getaddrinfo getpass gettext physmem poll strndup strsep sys_stat useless-if-before-free vasprintf vc-list-files AUTOMAKE_OPTIONS = 1.5 gnits @@ -701,6 +701,13 @@ EXTRA_DIST += unistd.in.h ## end gnulib module unistd +## begin gnulib module useless-if-before-free + + +EXTRA_DIST += $(top_srcdir)/build-aux/useless-if-before-free + +## end gnulib module useless-if-before-free + ## begin gnulib module vasnprintf diff --git a/gnulib/lib/vasnprintf.c b/gnulib/lib/vasnprintf.c index 205a9bf..ed579d3 100644 --- a/gnulib/lib/vasnprintf.c +++ b/gnulib/lib/vasnprintf.c @@ -3566,7 +3566,7 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, { arg_type type = a.arg[dp->arg_index].type; int flags = dp->flags; -#if !USE_SNPRINTF || !DCHAR_IS_TCHAR || ENABLE_UNISTDIO || NEED_PRINTF_FLAG_ZERO || NEED_PRINTF_UNBOUNDED_PRECISION +#if !USE_SNPRINTF || !DCHAR_IS_TCHAR || ENABLE_UNISTDIO || NEED_PRINTF_FLAG_LEFTADJUST || NEED_PRINTF_FLAG_ZERO || NEED_PRINTF_UNBOUNDED_PRECISION int has_width; size_t width; #endif @@ -3579,7 +3579,9 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, #else # define prec_ourselves 0 #endif -#if !DCHAR_IS_TCHAR || ENABLE_UNISTDIO || NEED_PRINTF_FLAG_ZERO || NEED_PRINTF_UNBOUNDED_PRECISION +#if NEED_PRINTF_FLAG_LEFTADJUST +# define pad_ourselves 1 +#elif !DCHAR_IS_TCHAR || ENABLE_UNISTDIO || NEED_PRINTF_FLAG_ZERO || NEED_PRINTF_UNBOUNDED_PRECISION int pad_ourselves; #else # define pad_ourselves 0 @@ -3593,7 +3595,7 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, TCHAR_T *tmp; #endif -#if !USE_SNPRINTF || !DCHAR_IS_TCHAR || ENABLE_UNISTDIO || NEED_PRINTF_FLAG_ZERO || NEED_PRINTF_UNBOUNDED_PRECISION +#if !USE_SNPRINTF || !DCHAR_IS_TCHAR || ENABLE_UNISTDIO || NEED_PRINTF_FLAG_LEFTADJUST || NEED_PRINTF_FLAG_ZERO || NEED_PRINTF_UNBOUNDED_PRECISION has_width = 0; width = 0; if (dp->width_start != dp->width_end) @@ -3883,7 +3885,7 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, #endif /* Decide whether to perform the padding ourselves. */ -#if !DCHAR_IS_TCHAR || ENABLE_UNISTDIO || NEED_PRINTF_FLAG_ZERO || NEED_PRINTF_UNBOUNDED_PRECISION +#if !NEED_PRINTF_FLAG_LEFTADJUST && (!DCHAR_IS_TCHAR || ENABLE_UNISTDIO || NEED_PRINTF_FLAG_ZERO || NEED_PRINTF_UNBOUNDED_PRECISION) switch (dp->conversion) { # if !DCHAR_IS_TCHAR || ENABLE_UNISTDIO @@ -4008,7 +4010,7 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, #endif *fbp = dp->conversion; #if USE_SNPRINTF -# if !(__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 3)) +# if !(__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 3) || ((defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__)) fbp[1] = '%'; fbp[2] = 'n'; fbp[3] = '\0'; @@ -4021,6 +4023,21 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, in format strings in writable memory may crash the program (if compiled with _FORTIFY_SOURCE=2), so we should avoid it in this situation. */ + /* On native Win32 systems (such as mingw), we can avoid using + %n because: + - Although the gl_SNPRINTF_TRUNCATION_C99 test fails, + snprintf does not write more than the specified number + of bytes. (snprintf (buf, 3, "%d %d", 4567, 89) writes + '4', '5', '6' into buf, not '4', '5', '\0'.) + - Although the gl_SNPRINTF_RETVAL_C99 test fails, snprintf + allows us to recognize the case of an insufficient + buffer size: it returns -1 in this case. + On native Win32 systems (such as mingw) where the OS is + Windows Vista, the use of %n in format strings by default + crashes the program. See + <http://gcc.gnu.org/ml/gcc/2007-06/msg00122.html> and + <http://msdn2.microsoft.com/en-us/library/ms175782(VS.80).aspx> + So we should avoid %n in this situation. */ fbp[1] = '\0'; # endif #else @@ -4494,7 +4511,7 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, /* Here count <= allocated - length. */ /* Perform padding. */ -#if !DCHAR_IS_TCHAR || ENABLE_UNISTDIO || NEED_PRINTF_FLAG_ZERO || NEED_PRINTF_UNBOUNDED_PRECISION +#if !DCHAR_IS_TCHAR || ENABLE_UNISTDIO || NEED_PRINTF_FLAG_LEFTADJUST || NEED_PRINTF_FLAG_ZERO || NEED_PRINTF_UNBOUNDED_PRECISION if (pad_ourselves && has_width) { size_t w; @@ -4535,15 +4552,14 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, # endif DCHAR_T *p = rp + count; DCHAR_T *end = p + pad; -# if NEED_PRINTF_FLAG_ZERO DCHAR_T *pad_ptr; -# if !DCHAR_IS_TCHAR +# if !DCHAR_IS_TCHAR if (dp->conversion == 'c' || dp->conversion == 's') /* No zero-padding for string directives. */ pad_ptr = NULL; else -# endif +# endif { pad_ptr = (*rp == '-' ? rp + 1 : rp); /* No zero-padding of "inf" and "nan". */ @@ -4551,7 +4567,6 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, || (*pad_ptr >= 'a' && *pad_ptr <= 'z')) pad_ptr = NULL; } -# endif /* The generated string now extends from rp to p, with the zero padding insertion point being at pad_ptr. */ @@ -4564,7 +4579,6 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, for (; pad > 0; pad--) *p++ = ' '; } -# if NEED_PRINTF_FLAG_ZERO else if ((flags & FLAG_ZERO) && pad_ptr != NULL) { /* Pad with zeroes. */ @@ -4575,7 +4589,6 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, for (; pad > 0; pad--) *p++ = '0'; } -# endif else { /* Pad with spaces on the left. */ diff --git a/gnulib/m4/gnulib-cache.m4 b/gnulib/m4/gnulib-cache.m4 index 357f170..50f743b 100644 --- a/gnulib/m4/gnulib-cache.m4 +++ b/gnulib/m4/gnulib-cache.m4 @@ -15,11 +15,11 @@ # Specification in the form of a command-line invocation: -# gnulib-tool --import --dir=. --lib=libgnu --source-base=gnulib/lib --m4-base=gnulib/m4 --doc-base=doc --aux-dir=build-aux --with-tests --lgpl=2 --libtool --macro-prefix=gl getaddrinfo getpass gettext physmem poll strndup strsep sys_stat vasprintf vc-list-files +# gnulib-tool --import --dir=. --lib=libgnu --source-base=gnulib/lib --m4-base=gnulib/m4 --doc-base=doc --aux-dir=build-aux --with-tests --lgpl=2 --libtool --macro-prefix=gl getaddrinfo getpass gettext physmem poll strndup strsep sys_stat useless-if-before-free vasprintf vc-list-files # Specification in the form of a few gnulib-tool.m4 macro invocations: gl_LOCAL_DIR([]) -gl_MODULES([getaddrinfo getpass gettext physmem poll strndup strsep sys_stat vasprintf vc-list-files]) +gl_MODULES([getaddrinfo getpass gettext physmem poll strndup strsep sys_stat useless-if-before-free vasprintf vc-list-files]) gl_AVOID([]) gl_SOURCE_BASE([gnulib/lib]) gl_M4_BASE([gnulib/m4]) diff --git a/gnulib/m4/gnulib-comp.m4 b/gnulib/m4/gnulib-comp.m4 index 8896a38..0eeee9c 100644 --- a/gnulib/m4/gnulib-comp.m4 +++ b/gnulib/m4/gnulib-comp.m4 @@ -225,6 +225,7 @@ AC_DEFUN([gltests_LIBSOURCES], [ AC_DEFUN([gl_FILE_LIST], [ build-aux/config.rpath build-aux/link-warning.h + build-aux/useless-if-before-free build-aux/vc-list-files lib/alloca.in.h lib/asnprintf.c diff --git a/gnulib/m4/vasnprintf.m4 b/gnulib/m4/vasnprintf.m4 index 6449a9c..c4d3f4f 100644 --- a/gnulib/m4/vasnprintf.m4 +++ b/gnulib/m4/vasnprintf.m4 @@ -1,5 +1,5 @@ -# vasnprintf.m4 serial 23 -dnl Copyright (C) 2002-2004, 2006-2007 Free Software Foundation, Inc. +# vasnprintf.m4 serial 24 +dnl Copyright (C) 2002-2004, 2006-2008 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, dnl with or without modifications, as long as this notice is preserved. @@ -166,6 +166,21 @@ AC_DEFUN([gl_PREREQ_VASNPRINTF_FLAG_GROUPING], esac ]) +# Extra prerequisites of lib/vasnprintf.c for supporting the '-' flag. +AC_DEFUN([gl_PREREQ_VASNPRINTF_FLAG_LEFTADJUST], +[ + AC_REQUIRE([gl_PRINTF_FLAG_LEFTADJUST]) + case "$gl_cv_func_printf_flag_leftadjust" in + *yes) + ;; + *) + AC_DEFINE([NEED_PRINTF_FLAG_LEFTADJUST], 1, + [Define if the vasnprintf implementation needs special code for the + '-' flag.]) + ;; + esac +]) + # Extra prerequisites of lib/vasnprintf.c for supporting the 0 flag. AC_DEFUN([gl_PREREQ_VASNPRINTF_FLAG_ZERO], [ @@ -234,6 +249,7 @@ AC_DEFUN([gl_PREREQ_VASNPRINTF_WITH_EXTRAS], gl_PREREQ_VASNPRINTF_DIRECTIVE_A gl_PREREQ_VASNPRINTF_DIRECTIVE_F gl_PREREQ_VASNPRINTF_FLAG_GROUPING + gl_PREREQ_VASNPRINTF_FLAG_LEFTADJUST gl_PREREQ_VASNPRINTF_FLAG_ZERO gl_PREREQ_VASNPRINTF_PRECISION gl_PREREQ_VASNPRINTF_ENOMEM -- 1.5.4.1.35.g55477