[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug-diffutils] Re: Bug#586301: diffutils: doubled slashes in --recursiv
From: |
Jim Meyering |
Subject: |
[bug-diffutils] Re: Bug#586301: diffutils: doubled slashes in --recursive output (path//path) (fwd) |
Date: |
Sun, 15 Aug 2010 00:26:21 +0200 |
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 <address@hidden>
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