Paul Eggert wrote: > On 08/14/10 23:46, Jim Meyering wrote: >> [PATCH] diff -r: avoid printing excess slashes in concatenated file names > > Thanks, that looks good to me. Hmm, at some point we should > replace zalloc with xzalloc too, I suppose, and maybe get > rid of diffutils' 'concat' function.
Hi Paul, Thanks for the quick feedback. FYI, my patch was relative to a slightly dated repository and no longer applied to "master". Also, it included an unnecessary (but harmless) mode change for an unrelated test. Better one below (which I'll push). When I rebased it to work on latest, I found that the result no longer builds with my compilation options: cc1: warnings being treated as errors dir.c: In function 'diff_dirs': dir.c:278: error: declaration of 'cmp' shadows a parameter [-Wshadow] dir.c:199: error: shadowed declaration is here [-Wshadow] make[2]: *** [dir.o] Error 1 Without thinking hard (meaning it might be better to change the shadowed parameter name, "cmp"), I made this local change to work around that: diff --git a/src/dir.c b/src/dir.c index 1b078ab..df89431 100644 --- a/src/dir.c +++ b/src/dir.c @@ -275,10 +275,10 @@ diff_dirs (struct comparison const *cmp, *p && compare_names (*p, greater_name) == 0; p++) { - int cmp = file_name_cmp (*p, greater_name); - if (0 <= cmp) + int c = file_name_cmp (*p, greater_name); + if (0 <= c) { - if (cmp == 0) + if (c == 0) { memmove (lesser + 1, lesser, (char *) p - (char *) lesser); >From 53de393ca335e77f22d3789100734c87868f12b3 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Sat, 14 Aug 2010 17:13:28 -0500 Subject: [PATCH] diff -r: avoid printing excess slashes in concatenated file names * bootstrap.conf (gnulib_modules): Add filenamecat. * src/diff.c: Include "filenamecat.h". (compare_files): Use file_name_concat, rather than dir_file_pathname. * src/util.c (dir_file_pathname): Remove now-unused function. * src/diff.h: Remove its declaration. * tests/excess-slash: New script to test for this. * tests/Makefile.am (TESTS): Add it. Forwarded by Santiago Vila from <bugs.debian.org/586301a>, reported by Jari Aalto. --- bootstrap.conf | 1 + gnulib | 2 +- src/diff.c | 7 ++++--- src/diff.h | 1 - src/util.c | 12 ------------ tests/Makefile.am | 3 ++- tests/excess-slash | 19 +++++++++++++++++++ 7 files changed, 27 insertions(+), 18 deletions(-) create mode 100644 tests/excess-slash diff --git a/bootstrap.conf b/bootstrap.conf index 2f104b3..ea1744c 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -33,6 +33,7 @@ extensions fcntl fdl file-type +filenamecat fnmatch-gnu getopt gettext diff --git a/gnulib b/gnulib index 880f2b6..0c6cf5a 160000 --- a/gnulib +++ b/gnulib @@ -1 +1 @@ -Subproject commit 880f2b69df57af506439d6aaf1fe185a6f960e43 +Subproject commit 0c6cf5ab43555377b99d94febb2d6f23fc3d2cb0 diff --git a/src/diff.c b/src/diff.c index cc1b611..807d38f 100644 --- a/src/diff.c +++ b/src/diff.c @@ -27,6 +27,7 @@ #include <error.h> #include <exclude.h> #include <exitfail.h> +#include <filenamecat.h> #include <file-type.h> #include <fnmatch.h> #include <getopt.h> @@ -1067,9 +1068,9 @@ compare_files (struct comparison const *parent, else { cmp.file[0].name = free0 - = dir_file_pathname (parent->file[0].name, name0); + = file_name_concat (parent->file[0].name, name0, NULL); cmp.file[1].name = free1 - = dir_file_pathname (parent->file[1].name, name1); + = file_name_concat (parent->file[1].name, name1, NULL); } /* Stat the files. */ @@ -1156,7 +1157,7 @@ compare_files (struct comparison const *parent, char const *fnm = cmp.file[fnm_arg].name; char const *dir = cmp.file[dir_arg].name; char const *filename = cmp.file[dir_arg].name = free0 - = dir_file_pathname (dir, last_component (fnm)); + = file_name_concat (dir, last_component (fnm), NULL); if (STREQ (fnm, "-")) fatal ("cannot compare `-' to a directory"); diff --git a/src/diff.h b/src/diff.h index 71b33f4..97f8d96 100644 --- a/src/diff.h +++ b/src/diff.h @@ -349,7 +349,6 @@ void print_sdiff_script (struct change *); extern char const change_letter[4]; extern char const pr_program[]; char *concat (char const *, char const *, char const *); -char *dir_file_pathname (char const *, char const *); bool lines_differ (char const *, char const *); lin translate_line_number (struct file_data const *, lin); struct change *find_change (struct change *); diff --git a/src/util.c b/src/util.c index 3be03e9..dd14e65 100644 --- a/src/util.c +++ b/src/util.c @@ -756,18 +756,6 @@ zalloc (size_t size) memset (p, 0, size); return p; } - -/* Yield the newly malloc'd pathname - of the file in DIR whose filename is FILE. */ - -char * -dir_file_pathname (char const *dir, char const *file) -{ - char const *base = last_component (dir); - size_t baselen = base_len (base); - bool omit_slash = baselen == 0 || base[baselen - 1] == '/'; - return concat (dir, "/" + omit_slash, file); -} void debug_script (struct change *sp) diff --git a/tests/Makefile.am b/tests/Makefile.am index 242d501..190f16a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,7 +1,8 @@ -TESTS = \ +jESTS = \ basic \ binary \ colliding-file-names \ + excess-slash \ help-version \ function-line-vs-leading-space \ label-vs-func \ diff --git a/tests/excess-slash b/tests/excess-slash new file mode 100644 index 0000000..3ef34cc --- /dev/null +++ b/tests/excess-slash @@ -0,0 +1,19 @@ +#!/bin/sh +# Ensure that no excess slash appears in diff -r output. + +: ${srcdir=.} +. "$srcdir/init.sh"; path_prepend_ ../src + +mkdir -p a/f b/f/g || framework_failure_ +echo Only in b/f: g > expected-out || framework_failure_ + +fail=0 + +diff -r a b/ > out 2> err && fail=1 + +# expect no stderr +compare err /dev/null || fail=1 + +compare out expected-out || fail=1 + +Exit $fail -- 1.7.2.1.186.gffe84 -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org