coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] stat: don't explicitly request file size for filenames


From: Jeff Layton
Subject: Re: [PATCH] stat: don't explicitly request file size for filenames
Date: Thu, 04 Jul 2019 06:37:56 -0400
User-agent: Evolution 3.32.3 (3.32.3-1.fc30)

On Wed, 2019-07-03 at 20:24 +0000, Andreas Dilger wrote:
> When calling 'stat -c %N' to print the filename, don't explicitly
> request the size of the file via statx(), as it may add overhead on
> some filesystems.  The size is only needed to optimize an allocation
> for the relatively rare case of reading a symlink name, and the worst
> effect is a somewhat-too-large temporary buffer may be allocated for
> areadlink_with_size(), or internal retries if buffer is too small.
> 
> The file size will be returned by statx() on most filesystems, even
> if not requested, unless the filesystem considers this to be too
> expensive for that file, in which case the tradeoff is worthwhile.
> 
> * src/stat.c: Don't explicitly request STATX_SIZE for filenames.
> Start with a 1KB buffer for areadlink_with_size() if st_size unset.
> ---
>  src/stat.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/stat.c b/src/stat.c
> index ec0bb7d..c887013 100644
> --- a/src/stat.c
> +++ b/src/stat.c
> @@ -1282,7 +1282,7 @@ fmt_to_mask (char fmt)
>    switch (fmt)
>      {
>      case 'N':
> -      return STATX_MODE|STATX_SIZE;
> +      return STATX_MODE;
>      case 'd':
>      case 'D':
>        return STATX_MODE;
> @@ -1491,7 +1491,9 @@ print_stat (char *pformat, size_t prefix_len, unsigned 
> int m,
>        out_string (pformat, prefix_len, quoteN (filename));
>        if (S_ISLNK (statbuf->st_mode))
>          {
> -          char *linkname = areadlink_with_size (filename, statbuf->st_size);
> +          /* if statx() didn't set size, most symlinks are under 1KB */
> +          char *linkname = areadlink_with_size (filename, statbuf->st_size ?:
> +                                                1023);
>            if (linkname == NULL)
>              {
>                error (0, errno, _("cannot read symbolic link %s"),

Looks reasonable to me.

Reviewed-by: Jeff Layton <address@hidden>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]