[PATCH 0/6] pull: rename fixes

[PATCH 0/6] pull: rename fixes

[Sami Kerola]

Hello,

Here are the rename(1) changes again.  Last time changing directory end
up to make merging not preferred, so I added ts_cd() function that does
everything I could think of ensuring directory change is successful.

On top of the previous patch set there are also two new rename(1)
changes.  Either the first four patches are merged now and the last two
later, or the whole thing should wait until final v2.25 release is out. 
My feeling later is the correct decision.


The following changes since commit d121efdb6e7bfe57d91d7ac46c3a3e13e9506584:
  cfdisk: add --zero command line option (2014-06-20 12:17:53 +0200)
are available in the git repository at:
  git://github.com/kerolasa/lelux-utiliteetit.git
for you to fetch changes up to d14036b0793c8bdddb89680fcfbb80b19b49e6ca:
  rename: use function pointer to select file or s

Sami Kerola (6):
  rename: allow renaming in subdirectories
  tests: add function to change directory reliable way
  tests: use ts_cd everywhere to change direcrory
  tests: add rename(1) checks
  rename: continue despite something failed
  rename: use function pointer to select file or symlink operation

 misc-utils/rename.c               | 124 ++++++++++++++++++++------------------
 tests/commands.sh                 |   1 +
 tests/expected/rename/basic       |   4 ++
 tests/expected/rename/subdir      |  20 ++++++
 tests/expected/rename/symlink     |   3 +
 tests/functions.sh                |  14 +++++
 tests/ts/build-sys/config         |   6 +-
 tests/ts/column/fillrow           |   2 +-
 tests/ts/column/invalid-multibyte |   2 +-
 tests/ts/column/multi-file        |   2 +-
 tests/ts/column/separator_table   |   2 +-
 tests/ts/cramfs/mkfs              |   8 +--
 tests/ts/namei/logic              |   2 +-
 tests/ts/rename/basic             |  41 +++++++++++++
 tests/ts/rename/subdir            |  42 +++++++++++++
 tests/ts/rename/symlink           |  41 +++++++++++++
 16 files changed, 245 insertions(+), 69 deletions(-)
 create mode 100644 tests/expected/rename/basic
 create mode 100644 tests/expected/rename/subdir
 create mode 100644 tests/expected/rename/symlink
 create mode 100755 tests/ts/rename/basic
 create mode 100755 tests/ts/rename/subdir
 create mode 100755 tests/ts/rename/symlink

-- 
2.0.0

[PATCH 1/6] rename: allow renaming in subdirectories

[Sami Kerola]

Earlier the rename(1) considered path as possible string to be renamed,
could lead to an issue with none existing destination.  See below for
demonstration of this issue.  After this change all directory elements
are ignored when the match finding happens.

$ cd $(mktemp -d)
$ mkdir aa ab
$ touch a{a,b}/aa
$ rename -v a x */aa
rename: aa/aa: rename to xa/aa failed: No such file or directory

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/rename.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/misc-utils/rename.c b/misc-utils/rename.c
index 9db85fb..b945aaa 100644
--- a/misc-utils/rename.c
+++ b/misc-utils/rename.c
@@ -46,9 +46,14 @@ static int do_rename(char *from, char *to, char *s, int verbose, int symtarget)
 
 		target[sb.st_size] = '\0';
 		where = strstr(target, from);
-	} else
-		where = strstr(s, from);
+	} else {
+		char *file;
 
+		file = rindex(s, '/');
+		if (file == NULL)
+			file = s;
+		where = strstr(file, from);
+	}
 	if (where == NULL) {
 		free(target);
 		return 0;
-- 
2.0.0

[PATCH 2/6] tests: add function to change directory reliable way

[Sami Kerola]

Without arguments bash cd will move to $HOME. Ensure also that when
directory is assumed to be changed the current directory and intented
destination are the same location.

Reference: http://www.spinics.net/lists/util-linux-ng/msg09509.html
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 tests/functions.sh | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tests/functions.sh b/tests/functions.sh
index 2a50de7..6998343 100644
--- a/tests/functions.sh
+++ b/tests/functions.sh
@@ -31,6 +31,20 @@ function ts_canonicalize {
 	fi
 }
 
+function ts_cd {
+	if [ $# -eq 0 ]; then
+		ts_failed "ul_cd: not enough arguments"
+	fi
+	DEST=$(readlink -f "$1" 2>/dev/null)
+	if [ "x$DEST" = "x" ] || [ ! -d "$DEST" ]; then
+		ts_failed "ul_cd: $1: no such directory"
+	fi
+	cd "$DEST" 2>/dev/null || ts_failed "ul_cd: $1: cannot change directory"
+	if [ "$PWD" != "$DEST" ]; then
+		ts_failed "ul_cd: $PWD is not $DEST"
+	fi
+}
+
 function ts_report {
 	if [ "$TS_PARALLEL" == "yes" ]; then
 		echo "$TS_TITLE $1"
-- 
2.0.0

[PATCH 3/6] tests: use ts_cd everywhere to change direcrory

[Sami Kerola]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 4173 bytes --]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 tests/ts/build-sys/config         | 6 +++---
 tests/ts/column/fillrow           | 2 +-
 tests/ts/column/invalid-multibyte | 2 +-
 tests/ts/column/multi-file        | 2 +-
 tests/ts/column/separator_table   | 2 +-
 tests/ts/cramfs/mkfs              | 8 ++++----
 tests/ts/namei/logic              | 2 +-
 7 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/tests/ts/build-sys/config b/tests/ts/build-sys/config
index 5ae83a3..15b3551 100755
--- a/tests/ts/build-sys/config
+++ b/tests/ts/build-sys/config
@@ -19,7 +19,7 @@ config_gen_dir="$top_srcdir/tools"
 
 [ -n "$CFLAGS" ] && export CFLAGS="$CFLAGS"
 
-cd $top_builddir && make -j clean &> /dev/null
+ts_cd $top_builddir && make -j clean &> /dev/null
 
 for conf in $config_gen_dir/config-gen.d/*.conf; do
 	ts_init_subtest $(basename $conf | sed 's/\.conf//')
@@ -27,7 +27,7 @@ for conf in $config_gen_dir/config-gen.d/*.conf; do
 	opts=$(ul_get_configuration $conf)
 
 	olddir=$(pwd)
-	cd $top_builddir
+	ts_cd $top_builddir
 
 	./configure $opts &> /dev/null
 	make -j &> /dev/null
@@ -63,7 +63,7 @@ for conf in $config_gen_dir/config-gen.d/*.conf; do
 	[ -d tests/diff.save ]   && mv tests/diff.save   tests/diff
 	[ -d tests/output.save ] && mv tests/output.save tests/output
 
-	cd $olddir
+	ts_cd $olddir
 	ts_finalize_subtest
 done
 
diff --git a/tests/ts/column/fillrow b/tests/ts/column/fillrow
index 263a267..5700924 100755
--- a/tests/ts/column/fillrow
+++ b/tests/ts/column/fillrow
@@ -23,7 +23,7 @@ ts_init "$*"
 
 ts_check_test_command "$TS_CMD_COLUMN"
 
-cd $TS_OUTDIR
+ts_cd "$TS_OUTDIR"
 
 $TS_CMD_COLUMN -x -c 50 $TS_SELF/input >> $TS_OUTPUT 2>&1
 
diff --git a/tests/ts/column/invalid-multibyte b/tests/ts/column/invalid-multibyte
index 9935a38..a9e3710 100755
--- a/tests/ts/column/invalid-multibyte
+++ b/tests/ts/column/invalid-multibyte
@@ -23,7 +23,7 @@ ts_init "$*"
 
 ts_check_test_command "$TS_CMD_COLUMN"
 
-cd $TS_OUTDIR
+ts_cd "$TS_OUTDIR"
 
 printf "£\n" | LC_ALL=C $TS_CMD_COLUMN >> $TS_OUTPUT 2>&1
 
diff --git a/tests/ts/column/multi-file b/tests/ts/column/multi-file
index 76b8686..bc7b331 100755
--- a/tests/ts/column/multi-file
+++ b/tests/ts/column/multi-file
@@ -24,7 +24,7 @@ ts_init "$*"
 
 ts_check_test_command "$TS_CMD_COLUMN"
 
-cd $TS_OUTDIR
+ts_cd "$TS_OUTDIR"
 
 $TS_CMD_COLUMN -x -c 50 $TS_SELF/input \
 			$TS_SELF/input \
diff --git a/tests/ts/column/separator_table b/tests/ts/column/separator_table
index 8be1e20..7c5caae 100755
--- a/tests/ts/column/separator_table
+++ b/tests/ts/column/separator_table
@@ -23,7 +23,7 @@ ts_init "$*"
 
 ts_check_test_command "$TS_CMD_COLUMN"
 
-cd $TS_OUTDIR
+ts_cd "$TS_OUTDIR"
 
 $TS_CMD_COLUMN -s 2 -t $TS_SELF/input >> $TS_OUTPUT 2>&1
 
diff --git a/tests/ts/cramfs/mkfs b/tests/ts/cramfs/mkfs
index 06cdfbd..5a10ba9 100755
--- a/tests/ts/cramfs/mkfs
+++ b/tests/ts/cramfs/mkfs
@@ -53,7 +53,7 @@ if [ ! -d "$IMAGE_SRC" ]; then
 	done
 fi
 
-cd $IMAGE_SRC
+ts_cd "$IMAGE_SRC"
 
 ts_log "list checksums from original data"
 find -type f -exec md5sum {} \; | sort >> $TS_OUTPUT
@@ -63,7 +63,7 @@ ts_log "create cramfs image"
 $TS_CMD_MKCRAMFS -n $LABEL $IMAGE_SRC $IMAGE_PATH 2>&1 >> $TS_OUTPUT
 [ -s "$IMAGE_PATH" ] || ts_die "Cannot create $IMAGE_PATH"
 
-cd $TS_OUTDIR
+ts_cd "$TS_OUTDIR"
 
 ts_log "count MD5 from the image"
 md5sum $IMAGE_NAME 2>&1 | sort >> $TS_OUTPUT
@@ -82,7 +82,7 @@ ts_mount "cramfs" -r -L $LABEL $TS_MOUNTPOINT
 # check it
 ts_is_mounted $DEVICE || ts_die "Cannot find $DEVICE in /proc/mounts" $DEVICE
 
-cd $TS_MOUNTPOINT
+ts_cd "$TS_MOUNTPOINT"
 
 ts_log "list the image"
 export TZ='GMT-1'
@@ -93,7 +93,7 @@ ts_log "list checksums from new data"
 find . -type f -exec md5sum {} \; | sort >> $TS_OUTPUT
 echo >> $TS_OUTPUT
 
-cd $ORIGPWD
+ts_cd "$ORIGPWD"
 
 ts_log "umount the image"
 $TS_CMD_UMOUNT $DEVICE
diff --git a/tests/ts/namei/logic b/tests/ts/namei/logic
index df48b76..703cc10 100755
--- a/tests/ts/namei/logic
+++ b/tests/ts/namei/logic
@@ -23,7 +23,7 @@ ts_init "$*"
 
 ts_check_test_command "$TS_CMD_NAMEI"
 
-cd $TS_OUTDIR
+ts_cd "$TS_OUTDIR"
 
 mkdir -p namei1/namei2
 touch namei1/namei2/a namei1/namei2/b
-- 
2.0.0

[PATCH 4/6] tests: add rename(1) checks

[Sami Kerola]

Check basic file moves, symlink relinking, and both file moves and
symlinks when operations are have directory in destination path.

Reviewed-by: Ruediger Meier <sweet_f_a@gmx.de>
Reviewed-by: Karel Zak <kzak@redhat.com>
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 tests/commands.sh             |  1 +
 tests/expected/rename/basic   |  4 ++++
 tests/expected/rename/subdir  | 20 ++++++++++++++++++++
 tests/expected/rename/symlink |  3 +++
 tests/ts/rename/basic         | 41 +++++++++++++++++++++++++++++++++++++++++
 tests/ts/rename/subdir        | 42 ++++++++++++++++++++++++++++++++++++++++++
 tests/ts/rename/symlink       | 41 +++++++++++++++++++++++++++++++++++++++++
 7 files changed, 152 insertions(+)
 create mode 100644 tests/expected/rename/basic
 create mode 100644 tests/expected/rename/subdir
 create mode 100644 tests/expected/rename/symlink
 create mode 100755 tests/ts/rename/basic
 create mode 100755 tests/ts/rename/subdir
 create mode 100755 tests/ts/rename/symlink

diff --git a/tests/commands.sh b/tests/commands.sh
index 06450a3..f699ef1 100644
--- a/tests/commands.sh
+++ b/tests/commands.sh
@@ -62,6 +62,7 @@ TS_CMD_MKSWAP=${TS_CMD_MKSWAP:-"$top_builddir/mkswap"}
 TS_CMD_MOUNT=${TS_CMD_MOUNT:-"$top_builddir/mount"}
 TS_CMD_NAMEI=${TS_CMD_NAMEI-"$top_builddir/namei"}
 TS_CMD_PARTX=${TS_CMD_PARTX-"$top_builddir/partx"}
+TS_CMD_RENAME=${TS_CMD_RENAME-"$top_builddir/rename"}
 TS_CMD_REV=${TS_CMD_REV:-"$top_builddir/rev"}
 TS_CMD_SCRIPT=${TS_CMD_SCRIPT-"$top_builddir/script"}
 TS_CMD_SETARCH=${TS_CMD_SETARCH-"$top_builddir/setarch"}
diff --git a/tests/expected/rename/basic b/tests/expected/rename/basic
new file mode 100644
index 0000000..8a7a1d8
--- /dev/null
+++ b/tests/expected/rename/basic
@@ -0,0 +1,4 @@
+`rename_basic.1' -> `rename_test.1'
+`rename_basic.2' -> `rename_test.2'
+`rename_basic.3' -> `rename_test.3'
+what is rename_basic.? doing here?
diff --git a/tests/expected/rename/subdir b/tests/expected/rename/subdir
new file mode 100644
index 0000000..a5b3dbe
--- /dev/null
+++ b/tests/expected/rename/subdir
@@ -0,0 +1,20 @@
+== files ==
+`rename_aa/aa' -> `rename_aa/xa'
+`rename_ab/aa' -> `rename_ab/xa'
+rename_aa
+rename_aa/xa
+rename_ab
+rename_ab/xa
+== symlinks ==
+rename_aa/sublink.1: `rename/aa' -> `renxme/aa'
+rename_aa/sublink.2: `rename/aa' -> `renxme/aa'
+rename_aa/sublink.3: `rename/aa' -> `renxme/aa'
+rename_ab/sublink.1: `rename/aa' -> `renxme/aa'
+rename_ab/sublink.2: `rename/aa' -> `renxme/aa'
+rename_ab/sublink.3: `rename/aa' -> `renxme/aa'
+renxme/aa
+renxme/aa
+renxme/aa
+renxme/aa
+renxme/aa
+renxme/aa
diff --git a/tests/expected/rename/symlink b/tests/expected/rename/symlink
new file mode 100644
index 0000000..38e687f
--- /dev/null
+++ b/tests/expected/rename/symlink
@@ -0,0 +1,3 @@
+rename_slink.1: `old' -> `new'
+rename_slink.2: `old' -> `new'
+rename_slink.3: `old' -> `new'
diff --git a/tests/ts/rename/basic b/tests/ts/rename/basic
new file mode 100755
index 0000000..cf3629e
--- /dev/null
+++ b/tests/ts/rename/basic
@@ -0,0 +1,41 @@
+#!/bin/bash
+
+#
+# Copyright (C) 2014 Sami Kerola <kerolasa@iki.fi>
+#
+# This file is part of util-linux.
+#
+# This file 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 of the License, or
+# (at your option) any later version.
+#
+# This file 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.
+#
+TS_TOPDIR="${0%/*}/../.."
+TS_DESC="basic check"
+
+. $TS_TOPDIR/functions.sh
+ts_init "$*"
+
+ts_check_test_command "$TS_CMD_RENAME"
+ts_cd "$TS_OUTDIR"
+
+touch rename_basic.{1..3}
+$TS_CMD_RENAME -v basic test rename_basic.? >> $TS_OUTPUT 2>&1
+
+for i in rename_basic.?; do
+	echo "what is $i doing here?" >> $TS_OUTPUT
+done
+for i in rename_test.{1..3}; do
+	if [ ! -f $i ]; then
+		echo "file $i is missing" >> $TS_OUTPUT
+	else
+		rm -f $i
+	fi
+done
+
+ts_finalize
diff --git a/tests/ts/rename/subdir b/tests/ts/rename/subdir
new file mode 100755
index 0000000..f83b2bf
--- /dev/null
+++ b/tests/ts/rename/subdir
@@ -0,0 +1,42 @@
+#!/bin/bash
+
+#
+# Copyright (C) 2014 Sami Kerola <kerolasa@iki.fi>
+#
+# This file is part of util-linux.
+#
+# This file 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 of the License, or
+# (at your option) any later version.
+#
+# This file 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.
+#
+TS_TOPDIR="${0%/*}/../.."
+TS_DESC="subdir check"
+
+. $TS_TOPDIR/functions.sh
+ts_init "$*"
+
+ts_check_test_command "$TS_CMD_RENAME"
+ts_cd "$TS_OUTDIR"
+
+echo "== files ==" >> $TS_OUTPUT
+mkdir rename_a{a,b}
+touch rename_a{a,b}/aa
+$TS_CMD_RENAME -v a x rename_a?/aa >> $TS_OUTPUT 2>&1
+find rename_a{a,b} >> $TS_OUTPUT 2>&1
+
+echo "== symlinks ==" >> $TS_OUTPUT
+for i in rename_a{a,b}/sublink.{1..3}; do
+	ln -s rename/aa $i
+done
+$TS_CMD_RENAME -s -v a x rename_a{a,b}/sublink.? >> $TS_OUTPUT 2>&1
+readlink rename_a{a,b}/sublink.?  >> $TS_OUTPUT 2>&1
+
+rm -rf rename_a{a,b}
+
+ts_finalize
diff --git a/tests/ts/rename/symlink b/tests/ts/rename/symlink
new file mode 100755
index 0000000..67f4392
--- /dev/null
+++ b/tests/ts/rename/symlink
@@ -0,0 +1,41 @@
+#!/bin/bash
+
+#
+# Copyright (C) 2014 Sami Kerola <kerolasa@iki.fi>
+#
+# This file is part of util-linux.
+#
+# This file 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 of the License, or
+# (at your option) any later version.
+#
+# This file 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.
+#
+TS_TOPDIR="${0%/*}/../.."
+TS_DESC="symlink check"
+
+. $TS_TOPDIR/functions.sh
+ts_init "$*"
+
+ts_check_test_command "$TS_CMD_RENAME"
+ts_cd "$TS_OUTDIR"
+
+for i in rename_slink.{1..3}; do
+	ln -s old $i
+done
+
+$TS_CMD_RENAME -s -v old new rename_slink.? >> $TS_OUTPUT 2>&1
+
+for i in rename_slink.{1..3}; do
+	where="$(readlink $i)"
+	if [ "$where" != "new" ]; then
+		echo "error: $i points to $where" >> $TS_OUTPUT
+	fi
+	rm -f $i
+done
+
+ts_finalize
-- 
2.0.0

[PATCH 5/6] rename: continue despite something failed

[Sami Kerola]

Try to do all file operations even when one or some of them fail.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/rename.c | 55 ++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 20 deletions(-)

diff --git a/misc-utils/rename.c b/misc-utils/rename.c
index b945aaa..250070b 100644
--- a/misc-utils/rename.c
+++ b/misc-utils/rename.c
@@ -29,20 +29,26 @@ for i in $@; do N=`echo "$i" | sed "s/$FROM/$TO/g"`; mv "$i" "$N"; done
 
 static int do_rename(char *from, char *to, char *s, int verbose, int symtarget)
 {
-	char *newname, *where, *p, *q, *target = NULL;
-	int flen, tlen, slen;
+	char *newname = NULL, *where, *p, *q, *target = NULL;
+	int flen, tlen, slen, ret = 0;
 	struct stat sb;
 
 	if (symtarget) {
-		if (lstat(s, &sb) == -1)
-			err(EXIT_FAILURE, _("%s: lstat failed"), s);
-
-		if (!S_ISLNK(sb.st_mode))
-			errx(EXIT_FAILURE, _("%s: not a symbolic link"), s);
+		if (lstat(s, &sb) == -1) {
+			warn(_("%s: lstat failed"), s);
+			return 1;
+		}
+		if (!S_ISLNK(sb.st_mode)) {
+			warnx(_("%s: not a symbolic link"), s);
+			return 1;
+		}
 
 		target = xmalloc(sb.st_size + 1);
-		if (readlink(s, target, sb.st_size + 1) < 0)
-			err(EXIT_FAILURE, _("%s: readlink failed"), s);
+		if (readlink(s, target, sb.st_size + 1) < 0) {
+			warn(_("%s: readlink failed"), s);
+			ret = 1;
+			goto out;
+		}
 
 		target[sb.st_size] = '\0';
 		where = strstr(target, from);
@@ -82,22 +88,31 @@ static int do_rename(char *from, char *to, char *s, int verbose, int symtarget)
 	*q = 0;
 
 	if (symtarget) {
-		if (0 > unlink(s))
-			err(EXIT_FAILURE, _("%s: unlink failed"), s);
-		if (symlink(newname, s) != 0)
-			err(EXIT_FAILURE, _("%s: symlinking to %s failed"), s, newname);
+		if (0 > unlink(s)) {
+			warn(_("%s: unlink failed"), s);
+			ret = 1;
+			goto out;
+		}
+		if (symlink(newname, s) != 0) {
+			warn(_("%s: symlinking to %s failed"), s, newname);
+			ret = 1;
+			goto out;
+		}
 		if (verbose)
 			printf("%s: `%s' -> `%s'\n", s, target, newname);
 	} else {
-		if (rename(s, newname) != 0)
-			err(EXIT_FAILURE, _("%s: rename to %s failed"), s, newname);
+		if (rename(s, newname) != 0) {
+			warn(_("%s: rename to %s failed"), s, newname);
+			ret = 1;
+			goto out;
+		}
 		if (verbose)
 			printf("`%s' -> `%s'\n", s, newname);
 	}
-
+ out:
 	free(newname);
 	free(target);
-	return 1;
+	return ret;
 }
 
 static void __attribute__ ((__noreturn__)) usage(FILE * out)
@@ -119,7 +134,7 @@ static void __attribute__ ((__noreturn__)) usage(FILE * out)
 int main(int argc, char **argv)
 {
 	char *from, *to;
-	int i, c, symtarget=0, verbose = 0;
+	int i, c, ret = 0, symtarget = 0, verbose = 0;
 
 	static const struct option longopts[] = {
 		{"verbose", no_argument, NULL, 'v'},
@@ -163,7 +178,7 @@ int main(int argc, char **argv)
 	to = argv[1];
 
 	for (i = 2; i < argc; i++)
-		do_rename(from, to, argv[i], verbose, symtarget);
+		ret |= do_rename(from, to, argv[i], verbose, symtarget);
 
-	return EXIT_SUCCESS;
+	return ret;
 }
-- 
2.0.0

[PATCH 6/6] rename: use function pointer to select file or symlink operation

[Sami Kerola]

Add separate functions to different functionality, and add a function for
the stuff that is in common for both.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/rename.c | 140 ++++++++++++++++++++++++----------------------------
 1 file changed, 65 insertions(+), 75 deletions(-)

diff --git a/misc-utils/rename.c b/misc-utils/rename.c
index 250070b..9c6872f 100644
--- a/misc-utils/rename.c
+++ b/misc-utils/rename.c
@@ -27,94 +27,83 @@ for i in $@; do N=`echo "$i" | sed "s/$FROM/$TO/g"`; mv "$i" "$N"; done
 #include "c.h"
 #include "closestream.h"
 
-static int do_rename(char *from, char *to, char *s, int verbose, int symtarget)
+static int string_replace(char *from, char *to, char *s, char *orig, char **newname)
 {
-	char *newname = NULL, *where, *p, *q, *target = NULL;
-	int flen, tlen, slen, ret = 0;
-	struct stat sb;
-
-	if (symtarget) {
-		if (lstat(s, &sb) == -1) {
-			warn(_("%s: lstat failed"), s);
-			return 1;
-		}
-		if (!S_ISLNK(sb.st_mode)) {
-			warnx(_("%s: not a symbolic link"), s);
-			return 1;
-		}
-
-		target = xmalloc(sb.st_size + 1);
-		if (readlink(s, target, sb.st_size + 1) < 0) {
-			warn(_("%s: readlink failed"), s);
-			ret = 1;
-			goto out;
-		}
-
-		target[sb.st_size] = '\0';
-		where = strstr(target, from);
-	} else {
-		char *file;
-
-		file = rindex(s, '/');
-		if (file == NULL)
-			file = s;
-		where = strstr(file, from);
-	}
-	if (where == NULL) {
-		free(target);
-		return 0;
-	}
-
-	flen = strlen(from);
-	tlen = strlen(to);
-	if (symtarget) {
-		slen = strlen(target);
-		p = target;
-	} else {
-		slen = strlen(s);
-		p = s;
-	}
-	newname = xmalloc(tlen + slen + 1);
-
-	q = newname;
+	char *p, *q, *where;
+
+	where = strstr(s, from);
+	if (where == NULL)
+		return 1;
+	p = orig;
+	*newname = xmalloc(strlen(orig) + strlen(to) + 1);
+	q = *newname;
 	while (p < where)
 		*q++ = *p++;
 	p = to;
 	while (*p)
 		*q++ = *p++;
-	p = where + flen;
+	p = where + strlen(from);
 	while (*p)
 		*q++ = *p++;
 	*q = 0;
+	return 0;
+}
 
-	if (symtarget) {
-		if (0 > unlink(s)) {
-			warn(_("%s: unlink failed"), s);
-			ret = 1;
-			goto out;
-		}
-		if (symlink(newname, s) != 0) {
-			warn(_("%s: symlinking to %s failed"), s, newname);
-			ret = 1;
-			goto out;
-		}
-		if (verbose)
-			printf("%s: `%s' -> `%s'\n", s, target, newname);
-	} else {
-		if (rename(s, newname) != 0) {
-			warn(_("%s: rename to %s failed"), s, newname);
-			ret = 1;
-			goto out;
-		}
-		if (verbose)
-			printf("`%s' -> `%s'\n", s, newname);
+static int do_symlink(char *from, char *to, char *s, int verbose)
+{
+	char *newname = NULL, *target = NULL;
+	int ret = 0;
+	struct stat sb;
+
+	if (lstat(s, &sb) == -1) {
+		warn(_("%s: lstat failed"), s);
+		return 1;
+	}
+	if (!S_ISLNK(sb.st_mode)) {
+		warnx(_("%s: not a symbolic link"), s);
+		return 1;
+	}
+	target = xmalloc(sb.st_size + 1);
+	if (readlink(s, target, sb.st_size + 1) < 0) {
+		warn(_("%s: readlink failed"), s);
+		free(target);
+		return 1;
 	}
- out:
+	target[sb.st_size] = '\0';
+	if (string_replace(from, to, target, target, &newname))
+		/* nothing */ ;
+	else if (0 > unlink(s)) {
+		warn(_("%s: unlink failed"), s);
+		ret = 1;
+	} else if (symlink(newname, s) != 0) {
+		warn(_("%s: symlinking to %s failed"), s, newname);
+		ret = 1;
+	} else if (verbose)
+		printf("%s: `%s' -> `%s'\n", s, target, newname);
 	free(newname);
 	free(target);
 	return ret;
 }
 
+static int do_file(char *from, char *to, char *s, int verbose)
+{
+	char *newname = NULL, *file;
+	int ret = 0;
+
+	file = rindex(s, '/');
+	if (file == NULL)
+		file = s;
+	if (string_replace(from, to, file, s, &newname))
+		return 0;
+	else if (rename(s, newname) != 0) {
+		warn(_("%s: rename to %s failed"), s, newname);
+		ret = 1;
+	} else if (verbose)
+		printf("`%s' -> `%s'\n", s, newname);
+	free(newname);
+	return ret;
+}
+
 static void __attribute__ ((__noreturn__)) usage(FILE * out)
 {
 	fputs(USAGE_HEADER, out);
@@ -134,7 +123,8 @@ static void __attribute__ ((__noreturn__)) usage(FILE * out)
 int main(int argc, char **argv)
 {
 	char *from, *to;
-	int i, c, ret = 0, symtarget = 0, verbose = 0;
+	int i, c, ret = 0, verbose = 0;
+	int (*do_rename)(char *from, char *to, char *s, int verbose) = do_file;
 
 	static const struct option longopts[] = {
 		{"verbose", no_argument, NULL, 'v'},
@@ -155,7 +145,7 @@ int main(int argc, char **argv)
 			verbose = 1;
 			break;
 		case 's':
-			symtarget = 1;
+			do_rename = do_symlink;
 			break;
 		case 'V':
 			printf(UTIL_LINUX_VERSION);
@@ -178,7 +168,7 @@ int main(int argc, char **argv)
 	to = argv[1];
 
 	for (i = 2; i < argc; i++)
-		ret |= do_rename(from, to, argv[i], verbose, symtarget);
+		ret |= do_rename(from, to, argv[i], verbose);
 
 	return ret;
 }
-- 
2.0.0

Re: [PATCH 0/6] pull: rename fixes

[Karel Zak]

On Sun, Jun 22, 2014 at 11:31:51AM +0100, Sami Kerola wrote:
>  misc-utils/rename.c               | 124 ++++++++++++++++++++------------------
>  tests/commands.sh                 |   1 +
>  tests/expected/rename/basic       |   4 ++
>  tests/expected/rename/subdir      |  20 ++++++
>  tests/expected/rename/symlink     |   3 +
>  tests/functions.sh                |  14 +++++
>  tests/ts/build-sys/config         |   6 +-
>  tests/ts/column/fillrow           |   2 +-
>  tests/ts/column/invalid-multibyte |   2 +-
>  tests/ts/column/multi-file        |   2 +-
>  tests/ts/column/separator_table   |   2 +-
>  tests/ts/cramfs/mkfs              |   8 +--
>  tests/ts/namei/logic              |   2 +-
>  tests/ts/rename/basic             |  41 +++++++++++++
>  tests/ts/rename/subdir            |  42 +++++++++++++
>  tests/ts/rename/symlink           |  41 +++++++++++++
>  16 files changed, 245 insertions(+), 69 deletions(-)
>  create mode 100644 tests/expected/rename/basic
>  create mode 100644 tests/expected/rename/subdir
>  create mode 100644 tests/expected/rename/symlink
>  create mode 100755 tests/ts/rename/basic
>  create mode 100755 tests/ts/rename/subdir
>  create mode 100755 tests/ts/rename/symlink

 Note that I'm going to merge it after v2.25 release. Thanks.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 5/6] rename: continue despite something failed

[Karel Zak]

On Sun, Jun 22, 2014 at 11:31:56AM +0100, Sami Kerola wrote:
> Try to do all file operations even when one or some of them fail.

 It would be nice to have a special return code for situation when
 something failed. See for example "mount -a" of kill(1) where we have 

    #define KILL_EXIT_SOMEOK      64

 and add to man page:

       0      success
       1      failure
       64     partial success

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 5/6] rename: continue despite something failed

[Sami Kerola]

On 25 June 2014 13:11, Karel Zak <kzak@redhat.com> wrote:
> On Sun, Jun 22, 2014 at 11:31:56AM +0100, Sami Kerola wrote:
>> Try to do all file operations even when one or some of them fail.
>
>  It would be nice to have a special return code for situation when
>  something failed. See for example "mount -a" of kill(1) where we have
>
>     #define KILL_EXIT_SOMEOK      64
>
>  and add to man page:
>
>        0      success
>        1      failure
>        64     partial success

Hi Karel,

Good idea, and I extended it a little. I added four different exit values,
that are hopefully useful.

  0    all requested rename operations were successful
  1    all rename operations failed
  2    some rename operations failed
  4    nothing was renamed
  64   unanticipated error occurred

The changes can be found from my git repository below, and I will
submit the patches 0005 and 0006 again to email list. There is also
new change, 0007, that is a check the exit values are working as
expected.

--- snip
The following changes since commit 575718a04aa0c053875041dc387e360f2dcaa70d:
  fallocate: use O_CREAT only for the default behavior (2014-06-26
14:45:02 +0200)
are available in the git repository at:
  git://github.com/kerolasa/lelux-utiliteetit.git
for you to fetch changes up to 40cbaab8022ba9e0dfeb3358655c0631bd1143bc:
  tests: add rename(1) return value check (2014-06-28 19:11:30 +0100)
-- snip

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

Re: [PATCH 0/6] pull: rename fixes

[Karel Zak]

On Sun, Jun 22, 2014 at 11:31:51AM +0100, Sami Kerola wrote:
> Sami Kerola (6):
>   rename: allow renaming in subdirectories
>   tests: add function to change directory reliable way
>   tests: use ts_cd everywhere to change direcrory
>   tests: add rename(1) checks
>   rename: continue despite something failed
>   rename: use function pointer to select file or symlink operation

 Applied, thanks.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com