Hi Petter,

thanks for the review of my patch!

Petter Reinholdtsen wrote:
> [Axel Beckert]
> > +      # check for free disk size percentage and don't overstep it
> > +      AVAIL_KB=$(/bin/df --output=avail "$(dirname "${CONF_SWAPFILE}")" | 
> > tail -1)
> 
> Note, I suspect this use of 'dirname' is a bad idea.

Main idea here was to avoid the situation that df errors out if the
file does not yet exist:

→ /bin/df /var/swap
/bin/df: /var/swap: No such file or directory
→ echo $?
0

At least it doesn't seem to exit with non-zero, but I wouldn't bet on
that being on purpose and forever like this.

> 'df' can give different result for /some/path and /some/path/file,
> and you want the result for the latter. For example, if /some/path
> is a autofs mount point that is not yet mounted, I believe 'df' will
> report data for the dummy mount point and not the file system
> mounted when accessing /some/path/file.

Correct.

> Because of this, I would suggest to touch the file first and then
> run 'df' on it, to make sure df report the space on the file system
> where the file is located.

Wouldn't accessing "/var/." after doing e.g. a stat on the (possibly
non-existing) file suffice for that? At least my gut feeling says that
I should avoid write access before actually creating the file.

                Regards, Axel
-- 
 ,''`.  |  Axel Beckert <a...@debian.org>, http://people.debian.org/~abe/
: :' :  |  Debian Developer, ftp.ch.debian.org Admin
`. `'   |  4096R: 2517 B724 C5F6 CA99 5329  6E61 2FF9 CD59 6126 16B5
  `-    |  1024D: F067 EA27 26B9 C3FC 1486  202E C09E 1D89 9593 0EDE

Reply via email to