rm(1) sync

["Liam J. Foy" <liamfoy@xxxxxxxxxxxxx>]

Hey Guys,

The following patch syncs our rm(1) with FreeBSD. The commit log on
the patch will describe it. If there are any problems, please post back with
them. Everything looks ok to me, but as you all know things slip. If no
objections are recieved, it will be committed sometime tomorrow or late
tonight.

Sync with FreeBSD rm(1) with my modifications. Some things have been
left out of this sync for time being.

- Fix the leakage of a file descriptor in rm_overwrite for non-fatal
  errors(this is for rm -P).
- Fix the fatal malloc() error message.
- When the P flag is set (i.e. Overwrite regular files before deleting them),
  do only unlink the file if we could indeed overwrite the file. See
  http://www.freebsd.org/cgi/cvsweb.cgi/src/bin/rm/rm.c?rev=1.45&content-type=text/x-cvsweb-markup
  for more details.
- Fix poor wording in comment.
- When fts_read() cannot stat the file, it can't be unlinked.  At
  that case, don't display error message when -f flag is used.
- Kindly let the user know rm(1) could not overwrite non-regular
  files and continue to unlink.
- Static functions.
- Constify rw_overwrite() and check().
Index: rm.c
===================================================================
RCS file: /home/dcvs/src/bin/rm/rm.c,v
retrieving revision 1.8
diff -u -r1.8 rm.c
--- rm.c	18 Oct 2004 17:38:52 -0000	1.8
+++ rm.c	6 Nov 2004 10:48:51 -0000
@@ -54,13 +54,13 @@
 int rflag, Iflag;
 uid_t uid;
 
-int	check(char *, char *, struct stat *);
-int	check2(char **);
-void	checkdot(char **);
-void	rm_file(char **);
-void	rm_overwrite(char *, struct stat *);
-void	rm_tree(char **);
-void	usage(void);
+static int	check(const char *, const char *, struct stat *);
+static int	check2(char **);
+static void	checkdot(char **);
+static void	rm_file(char **);
+static int	rm_overwrite(const char *, struct stat *);
+static void	rm_tree(char **);
+static void	usage(void);
 
 /*
  * rm --
@@ -156,7 +156,7 @@
 	exit (eval);
 }
 
-void
+static void
 rm_tree(char **argv)
 {
 	FTS *fts;
@@ -196,10 +196,10 @@
 		case FTS_ERR:
 			errx(1, "%s: %s", p->fts_path, strerror(p->fts_errno));
 		case FTS_NS:
-			/*
-			 * FTS_NS: assume that if can't stat the file, it
-			 * can't be unlinked.
-			 */
+		       /*
+			* Assume that since fts_read() couldn't stat
+			* the file, it can't be unlinked.
+			*/
 			if (!needstat)
 				break;
 			if (!fflag || p->fts_errno != ENOENT) {
@@ -267,9 +267,18 @@
 				}
 				break;
 
+			case FTS_NS:
+			/*
+			 * Assume that since fts_read() couldn't stat
+			 * the file, it can't be unlinked.
+			 */
+				if (fflag)
+					continue;
+				/* FALLTHROUGH */
 			default:
 				if (Pflag)
-					rm_overwrite(p->fts_accpath, NULL);
+					if (!rm_overwrite(p->fts_accpath, NULL))
+						continue;
 				rval = unlink(p->fts_accpath);
 				if (rval == 0 || (fflag && errno == ENOENT)) {
 					if (rval == 0 && vflag)
@@ -287,7 +296,7 @@
 		err(1, "fts_read");
 }
 
-void
+static void
 rm_file(char **argv)
 {
 	struct stat sb;
@@ -335,7 +344,8 @@
 				rval = rmdir(f);
 			else {
 				if (Pflag)
-					rm_overwrite(f, &sb);
+					if (!rm_overwrite(f, &sb))
+						continue;
 				rval = unlink(f);
 			}
 		}
@@ -359,7 +369,7 @@
  * System V filesystem).  In a logging filesystem, you'll have to have
  * kernel support.
  */
-void
+static int 
 rm_overwrite(const char *file, struct stat *sbp)
 {
 	struct stat sb;
@@ -374,15 +384,17 @@
 			goto err;
 		sbp = &sb;
 	}
-	if (!S_ISREG(sbp->st_mode))
-		return;
+	if (!S_ISREG(sbp->st_mode)) {
+		warnx("%s: cannot overwrite a non-regular file", file);
+		return (1);
+	}
 	if ((fd = open(file, O_WRONLY, 0)) == -1)
 		goto err;
 	if (fstatfs(fd, &fsb) == -1)
 		goto err;
 	bsize = MAX(fsb.f_iosize, 1024);
 	if ((buf = malloc(bsize)) == NULL)
-		err(1, "malloc");
+		err(1, "%s malloc failed", file);
 
 #define	PASS(byte) {							\
 	memset(buf, byte, bsize);					\
@@ -401,18 +413,21 @@
 	PASS(0xff);
 	if (!fsync(fd) && !close(fd)) {
 		free(buf);
-		return;
+		return (1);
 	}
 
 err:	eval = 1;
 	if (buf)
 		free(buf);
+	if (fd != -1)
+		close(fd);
 	warn("%s", file);
+	return (0);
 }
 
 
-int
-check(char *path, char *name, struct stat *sp)
+static int
+check(const char *path, const char *name, struct stat *sp)
 {
 	int ch, first;
 	char modep[15], *flagsp;
@@ -426,8 +441,11 @@
 		 * talking to a terminal, ask.  Symbolic links are excluded
 		 * because their permissions are meaningless.  Check stdin_ok
 		 * first because we may not have stat'ed the file.
+		 * Also skip this check if the -P option was specified because
+  	         * we will not be able to overwrite file contents and will
+  	         * barf later.
 		 */
-		if (!stdin_ok || S_ISLNK(sp->st_mode) ||
+		if (!stdin_ok || S_ISLNK(sp->st_mode) || Pflag ||
 		    (!access(name, W_OK) &&
 		    !(sp->st_flags & (SF_APPEND|SF_IMMUTABLE)) &&
 		    (!(sp->st_flags & (UF_APPEND|UF_IMMUTABLE)) || !uid)))
@@ -451,7 +469,7 @@
 	return (first == 'y' || first == 'Y');
 }
 
-int
+static int
 check2(char **argv)
 {
 	struct stat st;
@@ -502,7 +520,7 @@
 }
 
 #define ISDOT(a)	((a)[0] == '.' && (!(a)[1] || ((a)[1] == '.' && !(a)[2])))
-void
+static void
 checkdot(char **argv)
 {
 	char *p, **save, **t;
@@ -526,7 +544,7 @@
 	}
 }
 
-void
+static void
 usage(void)
 {
 


-- 
- Liam J. Foy
liamfoy@xxxxxxxxxxxxx