Re: [PATCH] Add the statx function

[Carlos O'Donell <carlos@redhat.com>]

On 07/05/2018 02:59 PM, Florian Weimer wrote:
> On 07/05/2018 08:36 PM, Paul Eggert wrote:
>> Florian Weimer wrote:
>>
>>> +  *buf = (struct statx)
>>> +    {
>>> +      /* We copy everything from fstat64, which corresponds the basic
>>> +         fstat64.  */
>>> +      .stx_mask = STATX_BASIC_STATS,
>>
>> That assignment to *BUF unnecessarily clears all *BUF fields not mentioned. It should be a bit more efficient to have a separate assignment for each mentioned field, e.g., 'buf->stx_mask = STATX_BASIC_STATS;'.
> 
> Clearing all padding fields is part of the userspace interface.  It's described in the UAPI header file, and the kernel implements that.
> 
> I've updated the comment.
> 
> I also fixed a C&P mistake in the major/minor extraction.
> 
>>> +int statx (int __dirfd, const char *__path, int __flags,
>>> +           unsigned int __mask, struct statx *__buf)
>>> +  __THROW __nonnull ((2, 5));
>>
>> The two pointer parameters should both be declared with __restrict.
> 
> Okay, I've added __restrict qualifiers.
> 
> As Andreas suggested, I've added STATX_ALL and STATX__RESERVED.

I'm not reviewing this yet.

However, as glibc 2.28 release manager I OK the addition of statx
into the release if you can get it reviewed in the next couple of
business days.

-- 
Cheers,
Carlos.