2009-04-08 23:11:34 -0000, Martin Pitt:
> Stephane, I originally added -mindepth 1 to avoid deleting /var/crash/
Hi Martin, thanks for looking into that
note that rm -f wont remove directories, so it's only useful in
case /var/crash has been changed to a file or to avoid an error
message.
> itself in the cronjob. However, I think -type f -mindepth 1 -maxdepth 1
> is a safer way.
Yes, that's the non-portable equivalent of
find /var/crash/. ! -name . -prune -type f
> So for the security update I propose to change
>
> find /var/crash -mindepth 1 -mtime +7 -print0 | xargs -0 rm -f
> find /var/crash -mindepth 1 -empty -print0 | xargs -0 rm -f
>
> to
>
> find /var/crash -mindepth 1 -maxdepth 1 -type f -mtime +7 -print0 | xargs -0 rm -f
> find /var/crash -mindepth 1 -maxdepth 1 -type f -empty -print0 | xargs -0 rm -f
>
> for the sake of minimal and well-understood changes. I'll do the more
> standards friendly version you proposed upstream.
I forgot to mention that -print0 and xargs -0 were not standard
as well. To have the equivalent of -exec ... {} +, you also need
the (non standard) -r option to xargs to avoid running rm if no
file is found (though it's harmless)
is that it makes smaller rm command lines (so potentially calls
fewer rm commands), also it avoid problems if /var/crash is
renamed or removed in the mean time (though, if that happens,
you're in serious trouble already I suppose).
> (Disclaimer: completely untested yet, I just confirmed that -type f does
> not catch symlinks)
[...]
Even if it did, rm -f would not remove the target of the link
but the link itself and -empty doesn't select symlinks and
-mtime checks the modification time of the link, not its target
(unless the -L option (former -follow predicate) is provided).
Another concern one may have (but that may just be nitpicking):
that script may remove files that are not /owned/ by apport,
which may become a problem if another package is using that
directory. So you may want to add a -name '*.crash' or maybe use
a directory named after the package name.
2009-04-08 23:11:34 -0000, Martin Pitt:
> Stephane, I originally added -mindepth 1 to avoid deleting /var/crash/
Hi Martin, thanks for looking into that
note that rm -f wont remove directories, so it's only useful in
case /var/crash has been changed to a file or to avoid an error
message.
> itself in the cronjob. However, I think -type f -mindepth 1 -maxdepth 1
> is a safer way.
Yes, that's the non-portable equivalent of
find /var/crash/. ! -name . -prune -type f
> So for the security update I propose to change
>
> find /var/crash -mindepth 1 -mtime +7 -print0 | xargs -0 rm -f
> find /var/crash -mindepth 1 -empty -print0 | xargs -0 rm -f
>
> to
>
> find /var/crash -mindepth 1 -maxdepth 1 -type f -mtime +7 -print0 | xargs -0 rm -f
> find /var/crash -mindepth 1 -maxdepth 1 -type f -empty -print0 | xargs -0 rm -f
>
> for the sake of minimal and well-understood changes. I'll do the more
> standards friendly version you proposed upstream.
I forgot to mention that -print0 and xargs -0 were not standard
as well. To have the equivalent of -exec ... {} +, you also need
the (non standard) -r option to xargs to avoid running rm if no
file is found (though it's harmless)
Another advantage of
cd /var/crash &&
find . ! -name . -prune -type f \( -size 0 -o -mtime +7 \) \
-exec rm -f {} +
is that it makes smaller rm command lines (so potentially calls
fewer rm commands), also it avoid problems if /var/crash is
renamed or removed in the mean time (though, if that happens,
you're in serious trouble already I suppose).
> (Disclaimer: completely untested yet, I just confirmed that -type f does
> not catch symlinks)
[...]
Even if it did, rm -f would not remove the target of the link
but the link itself and -empty doesn't select symlinks and
-mtime checks the modification time of the link, not its target
(unless the -L option (former -follow predicate) is provided).
Another concern one may have (but that may just be nitpicking):
that script may remove files that are not /owned/ by apport,
which may become a problem if another package is using that
directory. So you may want to add a -name '*.crash' or maybe use
a directory named after the package name.
Best regards,
Stephane