[bug-diffutils] Re: Bug#586301: diffutils: doubled slashes in --recursive output (path//path) (fwd)

[Jim Meyering]

Santiago Vila wrote:
> Received this from the Debian BTS.
>
> (I've given the reply-to a suitable value).
>
> ---------- Forwarded message ----------
> From: Jari Aalto <address@hidden>
> To: Debian Bug Tracking System <address@hidden>
> Date: Fri, 18 Jun 2010 11:54:30 +0300
> Subject: Bug#586301: diffutils: doubled slashes in --recursive output
>     (path//path)
>
> Package: diffutils
> Version: 1:3.0-1
> Severity: minor
>
>
> If given a directory with a trailing slash (as it occurs after TAB
> completion in Shell):
>
>     $ diff -rq .  /etc/apache2/
>     Files ./apache2.conf and /etc/apache2//apache2.conf differ
>     Only in /etc/apache2//conf.d: ganglia-webfrontend
>     Only in /etc/apache2//conf.d: javascript-common.conf
>     Only in /etc/apache2//conf.d: munin
>
> SUGGESTION
>
> Strip trailing slashes from command line arguments before using them for
> output.

Thanks to both Santiago and Jari.

Here's a quick stand-alone demo:
Before it would do this:

  $ ./diff -r a b/
  Only in b//f: g
  [Exit 1]

Now it works how we'd like:

  $ ./diff -r a b/
  Only in b/f: g
  [Exit 1]


Here's a proposed patch:

>From 280d5a1e6fd399af2fa5c9d0cfb3d88878bc2380 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 14 Aug 2010 16:23:51 -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 +
 src/diff.c              |    7 ++++---
 src/diff.h              |    1 -
 src/util.c              |   12 ------------
 tests/Makefile.am       |    1 +
 tests/excess-slash      |   19 +++++++++++++++++++
 6 files changed, 25 insertions(+), 16 deletions(-)
 create mode 100755 tests/excess-slash
 mode change 100644 => 100755 tests/no-newline-at-eof

diff --git a/bootstrap.conf b/bootstrap.conf
index 71fbd3f..33448fb 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -33,6 +33,7 @@ extensions
 fcntl
 fdl
 file-type
+filenamecat
 fnmatch-gnu
 getopt
 gettext
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 6a4858c..5a5f0f4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1,6 +1,7 @@
 TESTS = \
   basic \
   binary \
+  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 100755
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
diff --git a/tests/no-newline-at-eof b/tests/no-newline-at-eof
old mode 100644
new mode 100755
--
1.7.2.1.186.gffe84