f24e1f40e

Denis Vlasenko

2006-10-21

FAILED Duration: 3s

Subject

cp: add support for -s, -l. Fix free(nonmalloc) bug. Add doc on POSIX's rules on -i and -f (insane!). ln: make "ln dangling_symlink new_link" work.

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

coreutils/cp.c +27 -45 coreutils/libcoreutils/cp_mv_stat.c +1 -3 coreutils/ln.c +23 -20 coreutils/mv.c +7 -6 include/libbb.h +4 -1 libbb/copy_file.c +126 -84

coreutils/cp.c+27 -45

@@ -7,8 +7,6 @@
	* Licensed under GPL v2 or later, see file LICENSE in this tarball for details.
	*/

-	/* BB_AUDIT SUSv3 defects - unsupported options -H, -L, and -P. */
-	/* BB_AUDIT GNU defects - only extension options supported are -a and -d. */
	/* http://www.opengroup.org/onlinepubs/007904975/utilities/cp.html */

	/* Mar 16, 2003 Manuel Novoa III ([email protected])
@@ -16,15 +14,6 @@
	* Size reduction.
	*/

-	#include <sys/types.h>
-	#include <sys/stat.h>
-	#include <unistd.h>
-	#include <fcntl.h>
-	#include <utime.h>
-	#include <errno.h>
-	#include <dirent.h>
-	#include <stdlib.h>
-	#include <assert.h>
	#include "busybox.h"
	#include "libcoreutils/coreutils.h"

@@ -38,31 +27,26 @@ int cp_main(int argc, char **argv)
	int d_flags;
	int flags;
	int status = 0;
+	enum {
+	OPT_a = 1 << (sizeof(FILEUTILS_CP_OPTSTR)-1),
+	OPT_r = 1 << (sizeof(FILEUTILS_CP_OPTSTR)),
+	OPT_P = 1 << (sizeof(FILEUTILS_CP_OPTSTR)+1),
+	OPT_H = 1 << (sizeof(FILEUTILS_CP_OPTSTR)+2),
+	OPT_L = 1 << (sizeof(FILEUTILS_CP_OPTSTR)+3),
+	};

-	flags = getopt32(argc, argv, "pdRfiarPHL");
-
-	if (flags & 32) {
-	flags \|= (FILEUTILS_PRESERVE_STATUS \| FILEUTILS_RECUR \| FILEUTILS_DEREFERENCE);
-	}
-	if (flags & 64) {
-	/* Make -r a synonym for -R,
-	* -r was marked as obsolete in SUSv3, but is included for compatibility
-	*/
-	flags \|= FILEUTILS_RECUR;
-	}
-	if (flags & 128) {
-	/* Make -P a synonym for -d,
-	* -d is the GNU option while -P is the POSIX 2003 option
-	*/
-	flags \|= FILEUTILS_DEREFERENCE;
-	}
+	// Soft- and hardlinking don't mix
+	// -P and -d are the same (-P is POSIX, -d is GNU)
+	// -r and -R are the same
+	// -a = -pdR
+	opt_complementary = "?:l--s:s--l:Pd:rR:apdR";
+	flags = getopt32(argc, argv, FILEUTILS_CP_OPTSTR "arPHL");
	/* Default behavior of cp is to dereference, so we don't have to do
	* anything special when we are given -L.
	* The behavior of -H is almost like -L, but not quite, so let's
	* just ignore it too for fun.
-	if (flags & 256 \|\| flags & 512) {
-	;
-	}
+	if (flags & OPT_L) ...
+	if (flags & OPT_H) ... // deref command-line params only
	*/

	flags ^= FILEUTILS_DEREFERENCE; /* The sense of this flag was reversed. */
@@ -78,33 +62,31 @@ int cp_main(int argc, char **argv)
	if (optind + 2 == argc) {
	s_flags = cp_mv_stat2(*argv, &source_stat,
	(flags & FILEUTILS_DEREFERENCE) ? stat : lstat);
-	if ((s_flags < 0) \|\| ((d_flags = cp_mv_stat(last, &dest_stat)) < 0)) {
-	exit(EXIT_FAILURE);
-	}
+	if (s_flags < 0)
+	return EXIT_FAILURE;
+	d_flags = cp_mv_stat(last, &dest_stat);
+	if (d_flags < 0)
+	return EXIT_FAILURE;
+
	/* ...if neither is a directory or... */
	if ( !((s_flags \| d_flags) & 2) \|\|
	/* ...recursing, the 1st is a directory, and the 2nd doesn't exist... */
-	/* ((flags & FILEUTILS_RECUR) && (s_flags & 2) && !d_flags) */
-	/* Simplify the above since FILEUTILS_RECUR >> 1 == 2. */
-	((((flags & FILEUTILS_RECUR) >> 1) & s_flags) && !d_flags)
+	((flags & FILEUTILS_RECUR) && (s_flags & 2) && !d_flags)
	) {
	/* ...do a simple copy. */
-	dest = last;
+	dest = xstrdup(last);
	goto DO_COPY; /* Note: optind+2==argc implies argv[1]==last below. */
	}
	}

	do {
	dest = concat_path_file(last, bb_get_last_path_component(*argv));
-	DO_COPY:
+	DO_COPY:
	if (copy_file(*argv, dest, flags) < 0) {
	status = 1;
	}
-	if (*++argv == last) {
-	break;
-	}
-	free((void *) dest);
-	} while (1);
+	free((void*)dest);
+	} while (*++argv != last);

-	exit(status);
+	return status;
	}

coreutils/libcoreutils/cp_mv_stat.c+1 -3

@@ -20,8 +20,6 @@
	*
	*/

-	#include <errno.h>
-	#include <sys/stat.h>
	#include "libbb.h"
	#include "coreutils.h"

@@ -29,7 +27,7 @@ int cp_mv_stat2(const char fn, struct stat fn_stat, stat_func sf)
	{
	if (sf(fn, fn_stat) < 0) {
	if (errno != ENOENT) {
-	bb_perror_msg("unable to stat `%s'", fn);
+	bb_perror_msg("unable to stat '%s'", fn);
	return -1;
	}
	return 0;

coreutils/ln.c+23 -20

@@ -49,36 +49,39 @@ int ln_main(int argc, char **argv)
	src = last;

	if (is_directory(src,
-	(flag & LN_NODEREFERENCE) ^ LN_NODEREFERENCE,
-	NULL)) {
+	(flag & LN_NODEREFERENCE) ^ LN_NODEREFERENCE,
+	NULL)) {
	src_name = xstrdup(*argv);
	src = concat_path_file(src, bb_get_last_path_component(src_name));
	free(src_name);
	src_name = src;
	}
	if (!(flag & LN_SYMLINK) && stat(*argv, &statbuf)) {
-	bb_perror_msg("%s", *argv);
-	status = EXIT_FAILURE;
-	free(src_name);
-	continue;
+	// coreutils: "ln dangling_symlink new_hardlink" works
+	if (lstat(*argv, &statbuf) \|\| !S_ISLNK(statbuf.st_mode)) {
+	bb_perror_msg("%s", *argv);
+	status = EXIT_FAILURE;
+	free(src_name);
+	continue;
+	}
	}

	if (flag & LN_BACKUP) {
-	char *backup;
-	backup = xasprintf("%s%s", src, suffix);
-	if (rename(src, backup) < 0 && errno != ENOENT) {
-	bb_perror_msg("%s", src);
-	status = EXIT_FAILURE;
-	free(backup);
-	continue;
-	}
+	char *backup;
+	backup = xasprintf("%s%s", src, suffix);
+	if (rename(src, backup) < 0 && errno != ENOENT) {
+	bb_perror_msg("%s", src);
+	status = EXIT_FAILURE;
	free(backup);
-	/*
-	* When the source and dest are both hard links to the same
-	* inode, a rename may succeed even though nothing happened.
-	* Therefore, always unlink().
-	*/
-	unlink(src);
+	continue;
+	}
+	free(backup);
+	/*
+	* When the source and dest are both hard links to the same
+	* inode, a rename may succeed even though nothing happened.
+	* Therefore, always unlink().
+	*/
+	unlink(src);
	} else if (flag & LN_FORCE) {
	unlink(src);
	}

coreutils/mv.c+7 -6

@@ -57,7 +57,8 @@ int mv_main(int argc, char **argv)
	argv += optind;

	if (optind + 2 == argc) {
-	if ((dest_exists = cp_mv_stat(last, &dest_stat)) < 0) {
+	dest_exists = cp_mv_stat(last, &dest_stat);
+	if (dest_exists < 0) {
	return 1;
	}

@@ -69,8 +70,8 @@ int mv_main(int argc, char **argv)

	do {
	dest = concat_path_file(last, bb_get_last_path_component(*argv));
-
-	if ((dest_exists = cp_mv_stat(dest, &dest_stat)) < 0) {
+	dest_exists = cp_mv_stat(dest, &dest_stat);
+	if (dest_exists < 0) {
	goto RET_1;
	}

@@ -79,7 +80,7 @@ DO_MOVE:
	if (dest_exists && !(flags & OPT_FILEUTILS_FORCE) &&
	((access(dest, W_OK) < 0 && isatty(0)) \|\|
	(flags & OPT_FILEUTILS_INTERACTIVE))) {
-	if (fprintf(stderr, "mv: overwrite `%s'? ", dest) < 0) {
+	if (fprintf(stderr, "mv: overwrite '%s'? ", dest) < 0) {
	goto RET_1; /* Ouch! fprintf failed! */
	}
	if (!bb_ask_confirmation()) {
@@ -92,7 +93,7 @@ DO_MOVE:

	if (errno != EXDEV \|\|
	(source_exists = cp_mv_stat(*argv, &source_stat)) < 1) {
-	bb_perror_msg("unable to rename `%s'", *argv);
+	bb_perror_msg("cannot rename '%s'", *argv);
	} else {
	if (dest_exists) {
	if (dest_exists == 3) {
@@ -107,7 +108,7 @@ DO_MOVE:
	}
	}
	if (unlink(dest) < 0) {
-	bb_perror_msg("cannot remove `%s'", dest);
+	bb_perror_msg("cannot remove '%s'", dest);
	goto RET_1;
	}
	}

include/libbb.h+4 -1

@@ -473,8 +473,11 @@ enum { /* DO NOT CHANGE THESE VALUES! cp.c depends on them. */
	FILEUTILS_DEREFERENCE = 2,
	FILEUTILS_RECUR = 4,
	FILEUTILS_FORCE = 8,
-	FILEUTILS_INTERACTIVE = 16
+	FILEUTILS_INTERACTIVE = 0x10,
+	FILEUTILS_MAKE_HARDLINK = 0x20,
+	FILEUTILS_MAKE_SOFTLINK = 0x40,
	};
+	#define FILEUTILS_CP_OPTSTR "pdRfils"

	extern const char *applet_name;

libbb/copy_file.c+126 -84

@@ -10,31 +10,53 @@

	#include "libbb.h"

+	static int retry_overwrite(const char *dest, int flags)
+	{
+	if (!(flags & (FILEUTILS_FORCE\|FILEUTILS_INTERACTIVE))) {
+	fprintf(stderr, "'%s' exists\n", dest);
+	return -1;
+	}
+	if (flags & FILEUTILS_INTERACTIVE) {
+	fprintf(stderr, "%s: overwrite '%s'? ", applet_name, dest);
+	if (!bb_ask_confirmation())
+	return 0; // not allowed to overwrite
+	}
+	if (unlink(dest) < 0) {
+	bb_perror_msg("cannot remove '%s'", dest);
+	return -1; // error
+	}
+	return 1; // ok (to try again)
+	}
+
	int copy_file(const char source, const char dest, int flags)
	{
	struct stat source_stat;
	struct stat dest_stat;
-	int dest_exists = 0;
	int status = 0;
+	signed char dest_exists = 0;
+	signed char ovr;

-	if ((!(flags & FILEUTILS_DEREFERENCE) &&
-	lstat(source, &source_stat) < 0) \|\|
-	((flags & FILEUTILS_DEREFERENCE) &&
-	stat(source, &source_stat) < 0)) {
-	bb_perror_msg("%s", source);
+	#define FLAGS_DEREF (flags & FILEUTILS_DEREFERENCE)
+
+	if ((FLAGS_DEREF ? stat : lstat)(source, &source_stat) < 0) {
+	// This may be a dangling symlink.
+	// Making [sym]links to dangling symlinks works, so...
+	if (flags & (FILEUTILS_MAKE_SOFTLINK\|FILEUTILS_MAKE_HARDLINK))
+	goto make_links;
+	bb_perror_msg("cannot stat '%s'", source);
	return -1;
	}

	if (lstat(dest, &dest_stat) < 0) {
	if (errno != ENOENT) {
-	bb_perror_msg("unable to stat `%s'", dest);
+	bb_perror_msg("cannot stat '%s'", dest);
	return -1;
	}
	} else {
-	if (source_stat.st_dev == dest_stat.st_dev &&
-	source_stat.st_ino == dest_stat.st_ino)
-	{
-	bb_error_msg("`%s' and `%s' are the same file", source, dest);
+	if (source_stat.st_dev == dest_stat.st_dev
+	&& source_stat.st_ino == dest_stat.st_ino
+	) {
+	bb_error_msg("'%s' and '%s' are the same file", source, dest);
	return -1;
	}
	dest_exists = 1;
@@ -46,14 +68,14 @@ int copy_file(const char source, const char dest, int flags)
	mode_t saved_umask = 0;

	if (!(flags & FILEUTILS_RECUR)) {
-	bb_error_msg("%s: omitting directory", source);
+	bb_error_msg("omitting directory '%s'", source);
	return -1;
	}

	/* Create DEST. */
	if (dest_exists) {
	if (!S_ISDIR(dest_stat.st_mode)) {
-	bb_error_msg("`%s' is not a directory", dest);
+	bb_error_msg("target '%s' is not a directory", dest);
	return -1;
	}
	} else {
@@ -67,7 +89,7 @@ int copy_file(const char source, const char dest, int flags)

	if (mkdir(dest, mode) < 0) {
	umask(saved_umask);
-	bb_perror_msg("cannot create directory `%s'", dest);
+	bb_perror_msg("cannot create directory '%s'", dest);
	return -1;
	}

@@ -75,7 +97,8 @@ int copy_file(const char source, const char dest, int flags)
	}

	/* Recursively copy files in SOURCE. */
-	if ((dp = opendir(source)) == NULL) {
+	dp = opendir(source);
+	if (dp == NULL) {
	status = -1;
	goto preserve_status;
	}
@@ -84,7 +107,7 @@ int copy_file(const char source, const char dest, int flags)
	char new_source, new_dest;

	new_source = concat_subpath_file(source, d->d_name);
-	if(new_source == NULL)
+	if (new_source == NULL)
	continue;
	new_dest = concat_path_file(dest, d->d_name);
	if (copy_file(new_source, new_dest, flags) < 0)
@@ -92,103 +115,119 @@ int copy_file(const char source, const char dest, int flags)
	free(new_source);
	free(new_dest);
	}
-	/* closedir have only EBADF error, but "dp" not changes */
	closedir(dp);

-	if (!dest_exists &&
-	chmod(dest, source_stat.st_mode & ~saved_umask) < 0) {
-	bb_perror_msg("unable to change permissions of `%s'", dest);
+	if (!dest_exists
+	&& chmod(dest, source_stat.st_mode & ~saved_umask) < 0
+	) {
+	bb_perror_msg("cannot change permissions of '%s'", dest);
	status = -1;
	}
-	} else if (S_ISREG(source_stat.st_mode) \|\|
-	(S_ISLNK(source_stat.st_mode) && (flags & FILEUTILS_DEREFERENCE)))
-	{
+
+	} else if (flags & (FILEUTILS_MAKE_SOFTLINK\|FILEUTILS_MAKE_HARDLINK)) {
+	int (lf)(const char oldpath, const char *newpath);
+	make_links:
+	// Hmm... maybe
+	// if (DEREF && MAKE_SOFTLINK) source = realpath(source) ?
+	// (but realpath returns NULL on dangling symlinks...)
+	lf = (flags & FILEUTILS_MAKE_SOFTLINK) ? symlink : link;
+	if (lf(source, dest) < 0) {
+	ovr = retry_overwrite(dest, flags);
+	if (ovr <= 0)
+	return ovr;
+	if (lf(source, dest) < 0) {
+	bb_perror_msg("cannot create link '%s'", dest);
+	return -1;
+	}
+	}
+	return 0;
+
+	} else if (S_ISREG(source_stat.st_mode)
+	// Huh? DEREF uses stat, which never returns links IIRC...
+	\|\| (FLAGS_DEREF && S_ISLNK(source_stat.st_mode))
+	) {
	int src_fd;
	int dst_fd;
	if (ENABLE_FEATURE_PRESERVE_HARDLINKS) {
	char *link_name;

-	if (!(flags & FILEUTILS_DEREFERENCE) &&
-	is_in_ino_dev_hashtable(&source_stat, &link_name)) {
+	if (!FLAGS_DEREF
+	&& is_in_ino_dev_hashtable(&source_stat, &link_name)
+	) {
	if (link(link_name, dest) < 0) {
-	bb_perror_msg("unable to link `%s'", dest);
-	return -1;
+	ovr = retry_overwrite(dest, flags);
+	if (ovr <= 0)
+	return ovr;
+	if (link(link_name, dest) < 0) {
+	bb_perror_msg("cannot create link '%s'", dest);
+	return -1;
+	}
	}
-
	return 0;
	}
+	// TODO: probably is_in_.. and add_to_...
+	// can be combined: find_or_add_...
	add_to_ino_dev_hashtable(&source_stat, dest);
	}
+
	src_fd = open(source, O_RDONLY);
	if (src_fd == -1) {
-	bb_perror_msg("unable to open `%s'", source);
-	return(-1);
+	bb_perror_msg("cannot open '%s'", source);
+	return -1;
	}

-	if (dest_exists) {
-	if (flags & FILEUTILS_INTERACTIVE) {
-	fprintf(stderr, "%s: overwrite `%s'? ", applet_name, dest);
-	if (!bb_ask_confirmation()) {
-	close (src_fd);
-	return 0;
-	}
-	}
-
-	dst_fd = open(dest, O_WRONLY\|O_TRUNC);
-	if (dst_fd == -1) {
-	if (!(flags & FILEUTILS_FORCE)) {
-	bb_perror_msg("unable to open `%s'", dest);
-	close(src_fd);
-	return -1;
-	}
-
-	if (unlink(dest) < 0) {
-	bb_perror_msg("unable to remove `%s'", dest);
-	close(src_fd);
-	return -1;
-	}
-
-	goto dest_removed;
+	// POSIX: if exists and -i, ask (w/o -i assume yes).
+	// Then open w/o EXCL.
+	// If open still fails and -f, try unlink, then try open again.
+	// Result: a mess:
+	// If dest is a softlink, we overwrite softlink's destination!
+	// (or fail, if it points to dir/nonexistent location/etc).
+	// This is strange, but POSIX-correct.
+	// coreutils cp has --remove-destination to override this...
+	dst_fd = open(dest, (flags & FILEUTILS_INTERACTIVE)
+	? O_WRONLY\|O_CREAT\|O_TRUNC\|O_EXCL
+	: O_WRONLY\|O_CREAT\|O_TRUNC, source_stat.st_mode);
+	if (dst_fd == -1) {
+	// We would not do POSIX insanity. -i asks,
+	// then _unlinks_ the offender. Presto.
+	// Or else we will end up having 3 open()s!
+	ovr = retry_overwrite(dest, flags);
+	if (ovr <= 0) {
+	close(src_fd);
+	return ovr;
	}
-	} else {
-	dest_removed:
-	dst_fd = open(dest, O_WRONLY\|O_CREAT, source_stat.st_mode);
+	dst_fd = open(dest, O_WRONLY\|O_CREAT\|O_TRUNC, source_stat.st_mode);
	if (dst_fd == -1) {
-	bb_perror_msg("unable to open `%s'", dest);
+	bb_perror_msg("cannot open '%s'", dest);
	close(src_fd);
-	return(-1);
+	return -1;
	}
	}

	if (bb_copyfd_eof(src_fd, dst_fd) == -1)
	status = -1;
-
	if (close(dst_fd) < 0) {
-	bb_perror_msg("unable to close `%s'", dest);
+	bb_perror_msg("cannot close '%s'", dest);
	status = -1;
	}
-
	if (close(src_fd) < 0) {
-	bb_perror_msg("unable to close `%s'", source);
+	bb_perror_msg("cannot close '%s'", source);
	status = -1;
	}
-	} else if (S_ISBLK(source_stat.st_mode) \|\| S_ISCHR(source_stat.st_mode) \|\|
-	S_ISSOCK(source_stat.st_mode) \|\| S_ISFIFO(source_stat.st_mode) \|\|
-	S_ISLNK(source_stat.st_mode)) {

+	} else if (S_ISBLK(source_stat.st_mode) \|\| S_ISCHR(source_stat.st_mode)
+	\|\| S_ISSOCK(source_stat.st_mode) \|\| S_ISFIFO(source_stat.st_mode)
+	\|\| S_ISLNK(source_stat.st_mode)
+	) {
+	// We are lazy here, a bit lax with races...
	if (dest_exists) {
-	if((flags & FILEUTILS_FORCE) == 0) {
-	fprintf(stderr, "`%s' exists\n", dest);
-	return -1;
-	}
-	if(unlink(dest) < 0) {
-	bb_perror_msg("unable to remove `%s'", dest);
-	return -1;
-	}
+	ovr = retry_overwrite(dest, flags);
+	if (ovr <= 0)
+	return ovr;
	}
	if (S_ISFIFO(source_stat.st_mode)) {
	if (mkfifo(dest, source_stat.st_mode) < 0) {
-	bb_perror_msg("cannot create fifo `%s'", dest);
+	bb_perror_msg("cannot create fifo '%s'", dest);
	return -1;
	}
	} else if (S_ISLNK(source_stat.st_mode)) {
@@ -196,20 +235,21 @@ dest_removed:

	lpath = xreadlink(source);
	if (symlink(lpath, dest) < 0) {
-	bb_perror_msg("cannot create symlink `%s'", dest);
+	bb_perror_msg("cannot create symlink '%s'", dest);
+	free(lpath);
	return -1;
	}
	free(lpath);

	if (flags & FILEUTILS_PRESERVE_STATUS)
	if (lchown(dest, source_stat.st_uid, source_stat.st_gid) < 0)
-	bb_perror_msg("unable to preserve ownership of `%s'", dest);
+	bb_perror_msg("cannot preserve %s of '%s'", "ownership", dest);

	return 0;

	} else {
	if (mknod(dest, source_stat.st_mode, source_stat.st_rdev) < 0) {
-	bb_perror_msg("unable to create `%s'", dest);
+	bb_perror_msg("cannot create '%s'", dest);
	return -1;
	}
	}
@@ -218,22 +258,24 @@ dest_removed:
	return -1;
	}

-	preserve_status:
+	preserve_status:

-	if (flags & FILEUTILS_PRESERVE_STATUS) {
+	if (flags & FILEUTILS_PRESERVE_STATUS
+	/* Cannot happen: */
+	/* && !(flags & (FILEUTILS_MAKE_SOFTLINK\|FILEUTILS_MAKE_HARDLINK)) */
+	) {
	struct utimbuf times;
-	char *msg="unable to preserve %s of `%s'";

	times.actime = source_stat.st_atime;
	times.modtime = source_stat.st_mtime;
	if (utime(dest, &times) < 0)
-	bb_perror_msg(msg, "times", dest);
+	bb_perror_msg("cannot preserve %s of '%s'", "times", dest);
	if (chown(dest, source_stat.st_uid, source_stat.st_gid) < 0) {
	source_stat.st_mode &= ~(S_ISUID \| S_ISGID);
-	bb_perror_msg(msg, "ownership", dest);
+	bb_perror_msg("cannot preserve %s of '%s'", "ownership", dest);
	}
	if (chmod(dest, source_stat.st_mode) < 0)
-	bb_perror_msg(msg, "permissions", dest);
+	bb_perror_msg("cannot preserve %s of '%s'", "permissions", dest);
	}

	return status;