[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:43:38 -0400 |
User-agent: |
Evolution 3.32.3 (3.32.3-1.fc30) |
On Thu, 2019-07-04 at 06:37 -0400, Jeff Layton wrote:
> 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>
Actually, I'll retract that just yet...
I'm not sure that statbuf->st_size will be initialized to 0 if statx
didn't fill out the stx_size field. You may need to accompany this patch
with one that zeroes out the stx struct in do_stat.
--
Jeff Layton <address@hidden>
- [PATCH] stat: don't explicitly request file size for filenames, Andreas Dilger, 2019/07/03
- Re: [PATCH] stat: don't explicitly request file size for filenames, Jeff Layton, 2019/07/04
- Re: [PATCH] stat: don't explicitly request file size for filenames,
Jeff Layton <=
- Re: [PATCH] stat: don't explicitly request file size for filenames, Andreas Dilger, 2019/07/04
- Re: [PATCH] stat: don't explicitly request file size for filenames, Jeff Layton, 2019/07/05
- Re: [PATCH] stat: don't explicitly request file size for filenames, Jeff Layton, 2019/07/05
- Re: [PATCH] stat: don't explicitly request file size for filenames, Andreas Dilger, 2019/07/09
- Re: [PATCH] stat: don't explicitly request file size for filenames, Jeff Layton, 2019/07/10
- Re: [PATCH] stat: don't explicitly request file size for filenames, Pádraig Brady, 2019/07/12
Re: [PATCH] stat: don't explicitly request file size for filenames, Pádraig Brady, 2019/07/04