Re: /etc/daily: use find -delete

[Denis Fondras]

On Thu, Oct 08, 2020 at 05:32:15AM -0600, Todd C. Miller wrote:
> We can use find's built-in -delete primary to remove old /tmp files
> and directories.  This is somewhat less error-prone than execing
> rm or rmdir.
> 

OK denis@

>  - todd
> 
> Index: etc/daily
> ===
> RCS file: /cvs/src/etc/daily,v
> retrieving revision 1.93
> diff -u -p -u -r1.93 daily
> --- etc/daily 9 Sep 2019 20:02:26 -   1.93
> +++ etc/daily 22 Aug 2020 01:21:16 -
> @@ -50,17 +50,17 @@ if [ -d /tmp -a ! -L /tmp ]; then
>   find -x . \
>   \( -path './ssh-*' -o -path ./.X11-unix -o -path ./.ICE-unix \
>   -o -path './tmux-*' \) \
> - -prune -o -type f -atime +7 -execdir rm -f -- {} \; 2>/dev/null
> + -prune -o -type f -atime +7 -delete 2>/dev/null
>   find -x . -type d -mtime +1 ! -path ./vi.recover ! -path ./.X11-unix \
>   ! -path ./.ICE-unix ! -name . \
> - -execdir rmdir -- {} \; >/dev/null 2>&1; }
> + -delete >/dev/null 2>&1; }
>  fi
>  
>  # Additional junk directory cleanup would go like this:
>  #if [ -d /scratch -a ! -L /scratch ]; then
>  #cd /scratch && {
> -#find . ! -name . -atime +1 -execdir rm -f -- {} \;
> -#find . ! -name . -type d -mtime +1 -execdir rmdir -- {} \; \
> +#find . ! -name . -atime +1 -delete
> +#find . ! -name . -type d -mtime +1 -delete \
>  #>/dev/null 2>&1; }
>  #fi
>  
> 

/etc/daily: use find -delete

[Todd C . Miller]

We can use find's built-in -delete primary to remove old /tmp files
and directories.  This is somewhat less error-prone than execing
rm or rmdir.

 - todd

Index: etc/daily
===
RCS file: /cvs/src/etc/daily,v
retrieving revision 1.93
diff -u -p -u -r1.93 daily
--- etc/daily   9 Sep 2019 20:02:26 -   1.93
+++ etc/daily   22 Aug 2020 01:21:16 -
@@ -50,17 +50,17 @@ if [ -d /tmp -a ! -L /tmp ]; then
find -x . \
\( -path './ssh-*' -o -path ./.X11-unix -o -path ./.ICE-unix \
-o -path './tmux-*' \) \
-   -prune -o -type f -atime +7 -execdir rm -f -- {} \; 2>/dev/null
+   -prune -o -type f -atime +7 -delete 2>/dev/null
find -x . -type d -mtime +1 ! -path ./vi.recover ! -path ./.X11-unix \
! -path ./.ICE-unix ! -name . \
-   -execdir rmdir -- {} \; >/dev/null 2>&1; }
+   -delete >/dev/null 2>&1; }
 fi
 
 # Additional junk directory cleanup would go like this:
 #if [ -d /scratch -a ! -L /scratch ]; then
 #  cd /scratch && {
-#  find . ! -name . -atime +1 -execdir rm -f -- {} \;
-#  find . ! -name . -type d -mtime +1 -execdir rmdir -- {} \; \
+#  find . ! -name . -atime +1 -delete
+#  find . ! -name . -type d -mtime +1 -delete \
 #  >/dev/null 2>&1; }
 #fi
 

Re: find -delete

[Dmitrij D. Czarkoff]

"Ted Unangst"  wrote:

> This option is not posix (not like that's stopped find accumulating a
> dozen extensions), but it is in gnu and freebsd (for 20 years). it's
> also somewhat popular among sysadmins and blogs, etc. and perhaps most
> importantly, it nicely solves one of the more troublesome caveats of
> find (which the man page actually covers twice because it's so common
> and easy to screw up). So I think it makes a good addition.

I find this option highly distasteful and largerly useless.  You
generally want to use find -exec when you know what you are doing, so
choosing between ; and + is not really an issue unless you are doing it
wrong.  (Of course it might be an issue if you don't use find very
often, but then -delete is a huge hazard because of its nuances.)

I am OK with adding it for compatibility reasons, but I oppose adding
examples of its usage to manual page.

Re: find -delete

[Mark Kettenis]

> Date: Sat, 7 Jan 2017 13:15:05 +
> From: Stuart Henderson 
> 
> On 2017/01/07 11:32, Marc Espie wrote:
> > On Tue, Jan 03, 2017 at 11:15:39PM +0100, Mark Kettenis wrote:
> > > > From: "Ted Unangst" 
> > > > Date: Tue, 03 Jan 2017 16:39:48 -0500
> > > > 
> > > > I copied this straight from freebsd. Not fixed, but feel free to 
> > > > correct as
> > > > desired.
> > > > 
> > > > This adds a third example showing -delete, mentioning that it's not 
> > > > standard,
> > > > but also hinting that it may work better than "rm -r" when you want to 
> > > > delete
> > > > directories.
> > > 
> > > I really think we should not encourage unportable code like that by
> > > giving an example in our manual page.
> > > 
> > > I'm even tempted to say that you should leave the "-exec rm {} \;"
> > > example alone.  The + here only works because rm(1) accepts multiple
> > > file arguments.
> > 
> > Huh ? of course rm accepts multiple file arguments. All "good" unix commands
> > accept multiple file arguments, fortunately. That's what makes +exec (or
> > xargs for that matter) work.
> 
> I think the reason the example uses ; is probably because we only
> added + relatively recently (2012), and adding an example using it
> back then would make it harder for people writing scripts that
> work on newer and older OpenBSD. Enough time has passed that this
> isn't really an issue any more. But I would very much like to keep
> an example using ; to demonstrate the shell quoting needed when
> people have to use a bad command that only takes one file argument.

Right.  That was more or less my point.  The man page is there to
explain how find(1) works, not to show the most efficient way to
delete a bunch of files.

Not that a clear and concise discussion of the various trade-offs of
\; vs. + vs. -delete would hurt.

Re: find -delete

[Stuart Henderson]

On 2017/01/07 11:32, Marc Espie wrote:
> On Tue, Jan 03, 2017 at 11:15:39PM +0100, Mark Kettenis wrote:
> > > From: "Ted Unangst" 
> > > Date: Tue, 03 Jan 2017 16:39:48 -0500
> > > 
> > > I copied this straight from freebsd. Not fixed, but feel free to correct 
> > > as
> > > desired.
> > > 
> > > This adds a third example showing -delete, mentioning that it's not 
> > > standard,
> > > but also hinting that it may work better than "rm -r" when you want to 
> > > delete
> > > directories.
> > 
> > I really think we should not encourage unportable code like that by
> > giving an example in our manual page.
> > 
> > I'm even tempted to say that you should leave the "-exec rm {} \;"
> > example alone.  The + here only works because rm(1) accepts multiple
> > file arguments.
> 
> Huh ? of course rm accepts multiple file arguments. All "good" unix commands
> accept multiple file arguments, fortunately. That's what makes +exec (or
> xargs for that matter) work.

I think the reason the example uses ; is probably because we only
added + relatively recently (2012), and adding an example using it
back then would make it harder for people writing scripts that
work on newer and older OpenBSD. Enough time has passed that this
isn't really an issue any more. But I would very much like to keep
an example using ; to demonstrate the shell quoting needed when
people have to use a bad command that only takes one file argument.

Re: find -delete

[Marc Espie]

On Tue, Jan 03, 2017 at 11:15:39PM +0100, Mark Kettenis wrote:
> > From: "Ted Unangst" 
> > Date: Tue, 03 Jan 2017 16:39:48 -0500
> > 
> > I copied this straight from freebsd. Not fixed, but feel free to correct as
> > desired.
> > 
> > This adds a third example showing -delete, mentioning that it's not 
> > standard,
> > but also hinting that it may work better than "rm -r" when you want to 
> > delete
> > directories.
> 
> I really think we should not encourage unportable code like that by
> giving an example in our manual page.
> 
> I'm even tempted to say that you should leave the "-exec rm {} \;"
> example alone.  The + here only works because rm(1) accepts multiple
> file arguments.

Huh ? of course rm accepts multiple file arguments. All "good" unix commands
accept multiple file arguments, fortunately. That's what makes +exec (or
xargs for that matter) work.

Re: find -delete

[Ted Unangst]

Mark Kettenis wrote:
> I really think we should not encourage unportable code like that by
> giving an example in our manual page.

That's fair.

> I'm even tempted to say that you should leave the "-exec rm {} \;"
> example alone.  The + here only works because rm(1) accepts multiple
> file arguments.

This whole adventure started because I was trying to delete 10 old
emails. The default recipe, which execs one rm per file, was taking forever.
It would probably still be running. If I'm reading the find man page because
I'm trying to delete so many files I would overflow argv with shell wildcards,
I definitely want the turbo recipe.

> And I don't understand the sentence about recursively removing
> non-empty directories; rm(1) doesn't remove directories unless you use the
> -r option!

If some of the files you want to delete are directories, the obvious
solution when using xargs or -exec is to add -r to rm. But then this will
change behavior. I think it's possible to cook up a find invocation that uses
-type d -exec rmdir {} \; -o \! -type d -exec rm {} \; but as you see,
it gets unwieldy fast. I thought this would be worth highlighting.

Re: find -delete

[Theo de Raadt]

>> From: "Ted Unangst" 
>> Date: Tue, 03 Jan 2017 16:39:48 -0500
>> 
>> I copied this straight from freebsd. Not fixed, but feel free to correct as
>> desired.
>> 
>> This adds a third example showing -delete, mentioning that it's not standard,
>> but also hinting that it may work better than "rm -r" when you want to delete
>> directories.
>
>I really think we should not encourage unportable code like that by
>giving an example in our manual page.

Well lets be honest, we are accepting this feature last; it was only missing
in openbsd and whocaris

Re: find -delete

[Mark Kettenis]

> From: "Ted Unangst" 
> Date: Tue, 03 Jan 2017 16:39:48 -0500
> 
> I copied this straight from freebsd. Not fixed, but feel free to correct as
> desired.
> 
> This adds a third example showing -delete, mentioning that it's not standard,
> but also hinting that it may work better than "rm -r" when you want to delete
> directories.

I really think we should not encourage unportable code like that by
giving an example in our manual page.

I'm even tempted to say that you should leave the "-exec rm {} \;"
example alone.  The + here only works because rm(1) accepts multiple
file arguments.

And I don't understand the sentence about recursively removing
non-empty directories; rm(1) doesn't remove directories unless you use the
-r option!

> Index: find.1
> ===
> RCS file: /cvs/src/usr.bin/find/find.1,v
> retrieving revision 1.92
> diff -u -p -r1.92 find.1
> --- find.13 Jan 2017 21:31:16 -   1.92
> +++ find.13 Jan 2017 21:36:30 -
> @@ -575,9 +575,15 @@ ending in a dot and single digit, but sk
>  Find and remove all *.jpg and *.gif files under the current working
>  directory:
>  .Pp
> -.Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -exec rm {} \e;"
> +.Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -exec rm {} +"
>  or
>  .Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -print0 | xargs -0r rm"
> +or
> +.Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -delete"
> +.Pp
> +Note that this third version is not guaranteed to be portable.
> +However, if it is desired to delete directories as well, it may be safer
> +because it will not accidentally recursively remove non-emtpy directories.
>  .Sh SEE ALSO
>  .Xr chflags 1 ,
>  .Xr chmod 1 ,
> 
> 

Re: find -delete

[Ted Unangst]

Jason McIntyre wrote:
> no opinion on the addition, but if there is a better way to write the
> examples that are there, i think you should take the time to do so. i'd
> also slightly prefer we show the more traditional way to do it, though i
> appreciate that might not make a ton of sense for find(1).

Patch below.

> i'd use Sq rather than Dq for single letters if you really need it
> quoted. and check, but i think you need \& after the dot, not before, or
> you'll get a double space injected.

I copied this straight from freebsd. Not fixed, but feel free to correct as
desired.

This adds a third example showing -delete, mentioning that it's not standard,
but also hinting that it may work better than "rm -r" when you want to delete
directories.


Index: find.1
===
RCS file: /cvs/src/usr.bin/find/find.1,v
retrieving revision 1.92
diff -u -p -r1.92 find.1
--- find.1  3 Jan 2017 21:31:16 -   1.92
+++ find.1  3 Jan 2017 21:36:30 -
@@ -575,9 +575,15 @@ ending in a dot and single digit, but sk
 Find and remove all *.jpg and *.gif files under the current working
 directory:
 .Pp
-.Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -exec rm {} \e;"
+.Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -exec rm {} +"
 or
 .Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -print0 | xargs -0r rm"
+or
+.Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -delete"
+.Pp
+Note that this third version is not guaranteed to be portable.
+However, if it is desired to delete directories as well, it may be safer
+because it will not accidentally recursively remove non-emtpy directories.
 .Sh SEE ALSO
 .Xr chflags 1 ,
 .Xr chmod 1 ,

Re: find -delete

[Todd C. Miller]

OK millert@ for the code.  I'll defer to jmc@ on the man page bits.

 - todd

Re: find -delete

[Jason McIntyre]

On Tue, Jan 03, 2017 at 12:15:02AM -0500, Ted Unangst wrote:
> This option is not posix (not like that's stopped find accumulating a dozen
> extensions), but it is in gnu and freebsd (for 20 years). it's also somewhat
> popular among sysadmins and blogs, etc. and perhaps most importantly, it
> nicely solves one of the more troublesome caveats of find (which the man page
> actually covers twice because it's so common and easy to screw up). So I think
> it makes a good addition.
> 
> Code snatched from freebsd.
> 
> In passing, I'll also note that the man page example is very inefficient.
> $ find . \( -name \*.jpg -o -name \*.gif \) -exec rm {} \;
> This would be much faster with +. We can fix that too, but I'll add that a lot
> of the online advice suggests -delete, and if not available, -exec rm {} \;,
> instead of the smarter + exec. Not surprising since even the man page gets
> this wrong.
> 

no opinion on the addition, but if there is a better way to write the
examples that are there, i think you should take the time to do so. i'd
also slightly prefer we show the more traditional way to do it, though i
appreciate that might not make a ton of sense for find(1).

some comments on the man parts:

> Index: find.1
> ===
> RCS file: /cvs/src/usr.bin/find/find.1,v
> retrieving revision 1.91
> diff -u -p -r1.91 find.1
> --- find.111 Sep 2015 18:58:16 -  1.91
> +++ find.13 Jan 2017 05:04:52 -
> @@ -182,6 +182,23 @@ was started, rounded up to the next full
>  .Ar n
>  24-hour periods.
>  .Pp
> +.It Ic -delete
> +Delete found files and/or directories.

i think "and" is sufficiently clear.

> +Always returns true.
> +This executes
> +from the current working directory as
> +.Nm
> +recurses down the tree.
> +It will not attempt to delete a filename with a
> +.Dq Pa /
> +character in its pathname relative to
> +.Dq Pa \&.

i'd use Sq rather than Dq for single letters if you really need it
quoted. and check, but i think you need \& after the dot, not before, or
you'll get a double space injected.

jmc

> +for security reasons.
> +Depth-first traversal processing is implied by this option.
> +The
> +.Ic -delete
> +primary will fail to delete a directory if it is not empty.
> +Following symlinks is incompatible with this option.
>  .It Ic -depth
>  This primary always evaluates to true.
>  The same as specifying the
> @@ -587,6 +604,7 @@ primaries
>  .Ic -anewer ,
>  .Ic -cmin ,
>  .Ic -cnewer ,
> +.Ic -delete ,
>  .Ic -empty ,
>  .Ic -execdir ,
>  .Ic -flags ,

find -delete

[Ted Unangst]

This option is not posix (not like that's stopped find accumulating a dozen
extensions), but it is in gnu and freebsd (for 20 years). it's also somewhat
popular among sysadmins and blogs, etc. and perhaps most importantly, it
nicely solves one of the more troublesome caveats of find (which the man page
actually covers twice because it's so common and easy to screw up). So I think
it makes a good addition.

Code snatched from freebsd.

In passing, I'll also note that the man page example is very inefficient.
$ find . \( -name \*.jpg -o -name \*.gif \) -exec rm {} \;
This would be much faster with +. We can fix that too, but I'll add that a lot
of the online advice suggests -delete, and if not available, -exec rm {} \;,
instead of the smarter + exec. Not surprising since even the man page gets
this wrong.

Index: extern.h
===
RCS file: /cvs/src/usr.bin/find/extern.h,v
retrieving revision 1.21
diff -u -p -r1.21 extern.h
--- extern.h5 Oct 2015 15:25:16 -   1.21
+++ extern.h3 Jan 2017 04:57:04 -
@@ -53,6 +53,7 @@ PLAN  *c_atime(char *, char ***, int);
 PLAN   *c_cmin(char *, char ***, int);
 PLAN   *c_cnewer(char *, char ***, int);
 PLAN   *c_ctime(char *, char ***, int);
+PLAN   *c_delete(char *, char ***, int);
 PLAN   *c_depth(char *, char ***, int);
 PLAN   *c_empty(char *, char ***, int);
 PLAN   *c_exec(char *, char ***, int);
@@ -87,5 +88,5 @@ PLAN  *c_mtime(char *, char ***, int);
 PLAN   *c_not(char *, char ***, int);
 PLAN   *c_or(char *, char ***, int);
 
-extern int ftsoptions, isdepth, isoutput, isxargs;
+extern int ftsoptions, isdelete, isdepth, isoutput, isxargs;
 extern int mayexecve;
Index: find.1
===
RCS file: /cvs/src/usr.bin/find/find.1,v
retrieving revision 1.91
diff -u -p -r1.91 find.1
--- find.1  11 Sep 2015 18:58:16 -  1.91
+++ find.1  3 Jan 2017 05:04:52 -
@@ -182,6 +182,23 @@ was started, rounded up to the next full
 .Ar n
 24-hour periods.
 .Pp
+.It Ic -delete
+Delete found files and/or directories.
+Always returns true.
+This executes
+from the current working directory as
+.Nm
+recurses down the tree.
+It will not attempt to delete a filename with a
+.Dq Pa /
+character in its pathname relative to
+.Dq Pa \&.
+for security reasons.
+Depth-first traversal processing is implied by this option.
+The
+.Ic -delete
+primary will fail to delete a directory if it is not empty.
+Following symlinks is incompatible with this option.
 .It Ic -depth
 This primary always evaluates to true.
 The same as specifying the
@@ -587,6 +604,7 @@ primaries
 .Ic -anewer ,
 .Ic -cmin ,
 .Ic -cnewer ,
+.Ic -delete ,
 .Ic -empty ,
 .Ic -execdir ,
 .Ic -flags ,
Index: find.c
===
RCS file: /cvs/src/usr.bin/find/find.c,v
retrieving revision 1.20
diff -u -p -r1.20 find.c
--- find.c  10 Oct 2015 20:35:00 -  1.20
+++ find.c  3 Jan 2017 04:58:29 -
@@ -154,9 +154,15 @@ find_execute(PLAN *plan,   /* search plan 
int r, rval;
PLAN *p;
 
-   if (mayexecve == 0)
-   if (pledge("stdio rpath getpw", NULL) == -1)
-   err(1, "pledge");
+   if (mayexecve == 0) {
+   if (isdelete) {
+   if (pledge("stdio cpath rpath getpw", NULL) == -1)
+   err(1, "pledge");
+   } else {
+   if (pledge("stdio rpath getpw", NULL) == -1)
+   err(1, "pledge");
+   }
+   }
 
rval = 0;
 
Index: find.h
===
RCS file: /cvs/src/usr.bin/find/find.h,v
retrieving revision 1.17
diff -u -p -r1.17 find.h
--- find.h  15 Mar 2015 00:41:28 -  1.17
+++ find.h  3 Jan 2017 04:44:39 -
@@ -37,7 +37,7 @@
 enum ntype {
N_AND = 1,  /* must start > 0 */
N_AMIN, N_ANEWER, N_ATIME, N_CLOSEPAREN, N_CMIN, N_CNEWER, N_CTIME,
-   N_DEPTH, N_EMPTY, N_EXEC, N_EXECDIR, N_EXPR,
+   N_DELETE, N_DEPTH, N_EMPTY, N_EXEC, N_EXECDIR, N_EXPR,
N_FLAGS, N_FOLLOW, N_FSTYPE, N_GROUP, N_INAME, N_INUM, N_LINKS, N_LS,
N_MMIN, N_MAXDEPTH,
N_MINDEPTH, N_MTIME, N_NAME, N_NEWER, N_NOGROUP, N_NOT, N_NOUSER,
Index: function.c
===
RCS file: /cvs/src/usr.bin/find/function.c,v
retrieving revision 1.44
diff -u -p -r1.44 function.c
--- function.c  18 Apr 2015 18:28:37 -  1.44
+++ function.c  3 Jan 2017 05:00:20 -
@@ -295,6 +295,63 @@ c_depth(char *ignore, char ***ignored, i
 
return (palloc(N_DEPTH, f_always_true));
 }
+
+/*
+ * -delete funcions
+ */
+int
+f_delete(PLAN *plan, FTSENT *entry)
+{
+
+   /* can't delete these */
+   if (strcmp(entry->fts_accpath, ".") == 0 ||
+