linux-kernel - Re: Revised statx(2) man page for review

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 25 Apr 2017 20:50:33 +0200
From:   Silvan Jegen <s.jegen@...il.com>
To:     "Michael Kerrisk (man-pages)" <mtk.manpages@...il.com>
Cc:     David Howells <dhowells@...hat.com>,
        Jeff Layton <jlayton@...hat.com>,
        lkml <linux-kernel@...r.kernel.org>,
        Linux API <linux-api@...r.kernel.org>,
        linux-man <linux-man@...r.kernel.org>
Subject: Re: Revised statx(2) man page for review

Hi Michael

On Tue, Apr 25, 2017 at 01:14:26PM +0200, Michael Kerrisk (man-pages) wrote:
>
> [...]
>
> Could you please carefully review the text below, in case 
> I added any errors.

Just a few comments below.

 
> [...] 
>
>    Invoking statx():
>        To  access  a file's status, no permissions are required on the
>        file itself, but in the case of statx() with a  pathname,  exe‐
>        cute  (search) permission is required on all of the directories
>        in pathname that lead to the file.
> 
>        statx() uses pathname, dirfd, and  flags  identify  the  target

This should be:

"statx() uses pathname, dirfd, and  flags  *to* identify  the  target"


> [...] 
>
>        AT_STATX_SYNC_AS_STAT
>               Do  whatever  stat(2)  does.  This is the default and is
>               very much filesystem specific.

Since we write "Linux-specific" further above we should probably use

"very much filesystem-specific."

here for consistency.


>        AT_STATX_FORCE_SYNC
>               Force the attributes to be synchronized with the server.
>               This  may  require  that  a network filesystem perform a
>               data writeback to get the timestamps correct.
> 
>        AT_STATX_DONT_SYNC
>               Don't synchronize anything, but rather just  take  what‐
>               ever  the  system has cached if possible.  This may mean
>               that the information returned is approximate, but, on  a
>               network  filesystem,  it may not involve a round trip to
>               the server - even if no lease is held.
> 
>        The mask argument to statx() is used to tell the  kernel  which
>        fields  the  caller is interested in.  mask is an ORed combina‐
>        tion of the following constants:
> 
>            STATX_TYPE          Want stx_mode & S_IFMT
>            STATX_MODE          Want stx_mode & ~S_IFMT
>            STATX_NLINK         Want stx_nlink
>            STATX_UID           Want stx_uid
>            STATX_GID           Want stx_gid
>            STATX_ATIME         Want stx_atime
>            STATX_MTIME         Want stx_mtime
>            STATX_CTIME         Want stx_ctime
>            STATX_INO           Want stx_ino
>            STATX_SIZE          Want stx_size
>            STATX_BLOCKS        Want stx_blocks
>            STATX_BASIC_STATS   [All of the above]
>            STATX_BTIME         Want stx_btime
>            STATX_ALL           [All currently available fields]
> 
>        Note the kernel does not reject values in mask other  than  the

We should probably either insert a colon here

"Note: the kernel does not reject values in mask other  than  the..."

or reformulate the sentence like this.

"Note that the kernel does not reject values in mask other  than  the..."


> [...] 
>
>        stx_gid
>               The ID of the group that may access the file.

This group ID is the gid of the file owner's primary group, no? At least
that's what the field comment in the DESCRIPTION implies.

I think it would be more accurate to write:

"The ID of the file owner's primary group"


> [...] 
> 
>        STATX_ATTR_COMPRESSED
>               The  file  is  compressed  by  the fs and may take extra

We write out 'filesystem' everywhere else so I think we should replace 'fs'
with it here as well.


> CONFORMING TO
>        statx() is Linux specific.

For consistency we should write:

"statx() is Linux-specific".


I wrote the changes in-line but if you prefer I can 'git send-email'
a patch as well.


Cheers and thanks for all the hard work!

Silvan