From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 115485 invoked by alias); 5 Jul 2018 19:06:02 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 115466 invoked by uid 89); 5 Jul 2018 19:06:01 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=Hx-languages-length:1354, business X-HELO: mail-qk0-f195.google.com Return-Path: Subject: Re: [PATCH] Add the statx function To: Florian Weimer , Paul Eggert , libc-alpha@sourceware.org References: <20180630224103.4501543994575@oldenburg.str.redhat.com> <4934579e-a05c-3dcc-0b26-5ec09e03fbb3@cs.ucla.edu> From: Carlos O'Donell Openpgp: preference=signencrypt Message-ID: <4d3e7e55-fc34-bc3b-6d9e-afef45fc4d9e@redhat.com> Date: Thu, 05 Jul 2018 19:06:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2018-07/txt/msg00144.txt.bz2 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.