Some improvements to rm(1)

[Brooks Davis]

On Thu, Apr 25, 2013 at 10:16:32PM -0400, Eitan Adler wrote:
> Anyone have thoughts on the following?
> 
> commit 82c78ba923d8ce4a1bfbb309658c49021c8bb384
> Author: Eitan Adler <lists at eitanadler.com>
> Date:   Thu Apr 25 22:14:49 2013 -0400
> 
>     Take some improvements from DragonFlyBSD:
>     	- add const where appropriate
>     	- add static where appropriate
>     	- fix a whitespace issues

The no-op changes look more correct to me.

I think the -x option seems a bit odd.  What is the use case?  At a
first thought, it seems to raise more questions than it resolves.

-- Brooks

>     Obtained from:	DragonFlyBSD
> 
> diff --git a/bin/rm/rm.c b/bin/rm/rm.c
> index 1014921..660042b 100644
> --- a/bin/rm/rm.c
> +++ b/bin/rm/rm.c
> @@ -63,15 +63,15 @@ static int rflag, Iflag, xflag;
>  static uid_t uid;
>  static volatile sig_atomic_t info;
> 
> -int	check(char *, char *, struct stat *);
> -int	check2(char **);
> -void	checkdot(char **);
> -void	checkslash(char **);
> -void	rm_file(char **);
> -int	rm_overwrite(char *, struct stat *);
> -void	rm_tree(char **);
> +static int	check(const char *, const char *, struct stat *);
> +static int	check2(char **);
> +static void	checkdot(char **);
> +static void	checkslash(char **);
> +static void	rm_file(char **);
> +static int	rm_overwrite(const char *, struct stat *);
> +static void	rm_tree(char **);
>  static void siginfo(int __unused);
> -void	usage(void);
> +static void	usage(void);
> 
>  /*
>   * rm --
> @@ -173,7 +173,7 @@ main(int argc, char *argv[])
>  	exit (eval);
>  }
> 
> -void
> +static void
>  rm_tree(char **argv)
>  {
>  	FTS *fts;
> @@ -418,7 +418,7 @@ rm_file(char **argv)
>   * have kernel support.
>   */
>  int
> -rm_overwrite(char *file, struct stat *sbp)
> +rm_overwrite(const char *file, struct stat *sbp)
>  {
>  	struct stat sb, sb2;
>  	struct statfs fsb;
> @@ -484,8 +484,8 @@ err:	eval = 1;
>  }
> 
> 
> -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;
> @@ -496,7 +496,7 @@ check(char *path, char *name, struct stat *sp)
>  	else {
>  		/*
>  		 * If it's not a symbolic link and it's unwritable and we're
> -		 * talking to a terminal, ask.	Symbolic links are excluded
> +		 * 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.
>  		 */
> @@ -529,7 +529,7 @@ check(char *path, char *name, struct stat *sp)
>  }
> 
>  #define ISSLASH(a)	((a)[0] == '/' && (a)[1] == '\0')
> -void
> +static void
>  checkslash(char **argv)
>  {
>  	char **t, **u;
> @@ -549,7 +549,7 @@ checkslash(char **argv)
>  	}
>  }
> 
> -int
> +static int
>  check2(char **argv)
>  {
>  	struct stat st;
> @@ -600,7 +600,7 @@ check2(char **argv)
>  }
> 
>  #define ISDOT(a)	((a)[0] == '.' && (!(a)[1] || ((a)[1] == '.' && !(a)[2])))
> -void
> +static void
>  checkdot(char **argv)
>  {
>  	char *p, **save, **t;
> @@ -624,7 +624,7 @@ checkdot(char **argv)
>  	}
>  }
> 
> -void
> +static void
>  usage(void)
>  {
> 
> 
> commit 4799c480694b379f73483998e11341bffd3ffa3e
> Author: Eitan Adler <lists at eitanadler.com>
> Date:   Thu Apr 25 22:03:28 2013 -0400
> 
>     Add -x option to avoid crossing mount points when removing a hierarchy.
> 
>     Inspired by:	DragonflyBSD
> 
> diff --git a/bin/rm/rm.1 b/bin/rm/rm.1
> index 3588f38..824b627 100644
> --- a/bin/rm/rm.1
> +++ b/bin/rm/rm.1
> @@ -32,7 +32,7 @@
>  .\"	@(#)rm.1	8.5 (Berkeley) 12/5/94
>  .\" $FreeBSD$
>  .\"
> -.Dd March 15, 2013
> +.Dd April 25, 2013
>  .Dt RM 1
>  .Os
>  .Sh NAME
> @@ -42,7 +42,7 @@
>  .Sh SYNOPSIS
>  .Nm
>  .Op Fl f | i
> -.Op Fl dIPRrvW
> +.Op Fl dIPRrvWx
>  .Ar
>  .Nm unlink
>  .Ar file
> @@ -132,6 +132,8 @@ Attempt to undelete the named files.
>  Currently, this option can only be used to recover
>  files covered by whiteouts in a union file system (see
>  .Xr undelete 2 ) .
> +.It Fl x
> +When removing a hierarchy, do not cross mount points.
>  .El
>  .Pp
>  The
> diff --git a/bin/rm/rm.c b/bin/rm/rm.c
> index a450057..1014921 100644
> --- a/bin/rm/rm.c
> +++ b/bin/rm/rm.c
> @@ -59,7 +59,7 @@ __FBSDID("$FreeBSD$");
>  #include <unistd.h>
> 
>  static int dflag, eval, fflag, iflag, Pflag, vflag, Wflag, stdin_ok;
> -static int rflag, Iflag;
> +static int rflag, Iflag, xflag;
>  static uid_t uid;
>  static volatile sig_atomic_t info;
> 
> @@ -106,8 +106,8 @@ main(int argc, char *argv[])
>  		exit(eval);
>  	}
> 
> -	Pflag = rflag = 0;
> -	while ((ch = getopt(argc, argv, "dfiIPRrvW")) != -1)
> +	Pflag = rflag = xflag = 0;
> +	while ((ch = getopt(argc, argv, "dfiIPRrvWx")) != -1)
>  		switch(ch) {
>  		case 'd':
>  			dflag = 1;
> @@ -136,6 +136,9 @@ main(int argc, char *argv[])
>  		case 'W':
>  			Wflag = 1;
>  			break;
> +		case 'x':
> +			xflag = 1;
> +			break;
>  		default:
>  			usage();
>  		}
> @@ -196,6 +199,8 @@ rm_tree(char **argv)
>  		flags |= FTS_NOSTAT;
>  	if (Wflag)
>  		flags |= FTS_WHITEOUT;
> +	if (xflag)
> +		flags |= FTS_XDEV;
>  	if (!(fts = fts_open(argv, flags, NULL))) {
>  		if (fflag && errno == ENOENT)
>  			return;
> @@ -624,7 +629,7 @@ usage(void)
>  {
> 
>  	(void)fprintf(stderr, "%s\n%s\n",
> -	    "usage: rm [-f | -i] [-dIPRrvW] file ...",
> +	    "usage: rm [-f | -i] [-dIPRrvWx] file ...",
>  	    "       unlink file");
>  	exit(EX_USAGE);
>  }
> 
> 
> -- 
> Eitan Adler
> _______________________________________________
> freebsd-hackers at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe at freebsd.org"
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 188 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-hackers/attachments/20130425/b4f5544f/attachment.sig>