Re: [PATCH] fix descriptions for AT_NO_AUTOMOUNT

[Alejandro Colomar (man-pages)]

On 3/14/22 19:02, Pádraig Brady wrote:
> On 14/03/2022 13:24, Alejandro Colomar (man-pages) wrote:
>> On 3/10/22 14:46, Pádraig Brady wrote:
>>> Subject: [PATCH] fix descriptions for AT_NO_AUTOMOUNT
>>>
>>> Don't mention AT_NO_AUTOMOUNT for fstatat.2
>>> as it's implied since v4.11-rc7-14-gdeccf497d804
>>
>> Even though it's implied, since code may pass it,
>> and especially code written based on the old manual page,
>> it would be good to keep the paragraph in fstatat.2,
>> even if the text is replaced by something like
>> "Before Linux x.xx, this flag was ignored.
>> After Linux y.yy, this flag is implied."
>>
>> Does it make sense to you?
> 
> Yes good point.
> I went through the git history and the summary is fstatat()
> honored the flag since 2.6.38,
> ignored the flag since 3.1,
> implied the flag since 4.11,
> 
> I'll add that info to fstatat(2), and the details to the commit message.
> 
>>> Don't mention commit v4.13-9318-g42f461482178 as it was reverted
>>
>> Please also mention v4.15-rc1-50-g5d38f049cee1 as the commit in which it
>> was reverted.
>> Since it was present in some kernel releases, we might want to mention
>> it in the manual page?
> 
> Well since the flag for fstatat() doesn't change anything since 3.1
> it's probably best not to mention this old, short lived, and very
> specific info.

Okay.

> 
>>> Mention that stat(), lstat(), and fstatat()
>>> imply AT_NO_AUTOMOUNT, on the statx.2 man page
>>
>> Please sign the patch with "Signed-off-by: ..."
>> <https://www.kernel.org/doc/man-pages/patches.html>
> 
> Done in the attached.

Please send the patch inline (or both inline and attached),
if you don't mind.
However, if it's a big hassle for you,
I can apply it from an attachment.

> 
> thanks!
> Pádraig


---

> 
> Subject: [PATCH] fix descriptions for AT_NO_AUTOMOUNT
> 
> fstatat(..., AT_NO_AUTOMOUNT) has had the following history in Linux:
>   v2.6.37-7314-g6f45b65672c8
>     add AT_NO_AUTOMOUNT and fstatat honors it
>   v3.1-rc7-68-gb6c8069d3577
>     ignore AT_NO_AUTOMOUNT since default operation now less eagerly mounts
>   v4.10-11255-ga528d35e8bfc
>     adds statx which reinstated 2.6.38 behavior for fstatat (not released)
>   v4.11-rc7-14-gdeccf497d804
>     adjust fstatat so that AT_NO_AUTOMOUNT always specified (to statx)
> 
> * man2/stat.2:
> Adjust AT_NO_AUTOMOUNT description for fstatat.2 as per the above,
> to indicate AT_NO_AUTOMOUNT should be avoided with fstatat() since
> it's ignored since 3.1 and implied since 4.11.
> 
> Don't mention commit v4.13-9318-g42f461482178 as it was reverted,
> and moot anyway since we can't adjust AT_NO_AUTOMOUNT since 3.1.
> 
> * man2/statx.2:
> Mention that stat(), lstat(), and fstatat() imply AT_NO_AUTOMOUNT.
> 
> Signed-off-by: Pádraig Brady <P@draigBrady.com>

Makes sense to me.
But see a few minor comments below.

Thanks,

Alex

> ---
>  man2/stat.2  | 31 +++----------------------------
>  man2/statx.2 | 18 +++++++++++++++---
>  2 files changed, 18 insertions(+), 31 deletions(-)
> 
> diff --git a/man2/stat.2 b/man2/stat.2
> index 016c1f47d..9000b2ca6 100644
> --- a/man2/stat.2
> +++ b/man2/stat.2
> @@ -319,34 +319,9 @@ to obtain its definition.
>  .TP
>  .BR AT_NO_AUTOMOUNT " (since Linux 2.6.38)"
>  Don't automount the terminal ("basename") component of
> -.I pathname
> -if it is a directory that is an automount point.
> -This allows the caller to gather attributes of an automount point
> -(rather than the location it would mount).
> -Since Linux 4.14,
> -.\" commit 42f46148217865a545e129612075f3d828a2c4e4
> -also don't instantiate a nonexistent name in an
> -on-demand directory such as used for automounter indirect maps.
> -This
> -flag has no effect if the mount point has already been mounted over.
> -.IP
> -Both
> -.BR stat ()
> -and
> -.BR lstat ()
> -act as though
> -.B AT_NO_AUTOMOUNT
> -was set.
> -.IP
> -The
> -.B AT_NO_AUTOMOUNT
> -can be used in tools that scan directories
> -to prevent mass-automounting of a directory of automount points.
> -.IP
> -This flag is Linux-specific; define
> -.B _GNU_SOURCE
> -.\" Before glibc 2.16, defining _ATFILE_SOURCE sufficed
> -to obtain its definition.
> +.I pathname.
> +Since Linux 3.1 this flag is ignored.
> +Since Linux 4.11 this flag is implied.
>  .TP
>  .B AT_SYMLINK_NOFOLLOW
>  If
> diff --git a/man2/statx.2 b/man2/statx.2
> index 04b3e5075..d4e638756 100644
> --- a/man2/statx.2
> +++ b/man2/statx.2
> @@ -195,11 +195,23 @@ Don't automount the terminal ("basename") component of
>  if it is a directory that is an automount point.
>  This allows the caller to gather attributes of an automount point
>  (rather than the location it would mount).
> -This flag can be used in tools that scan directories
> -to prevent mass-automounting of a directory of automount points.
> +This
> +flag has no effect if the mount point has already been mounted over.

Weird break point there.
I see that it was like that in existing code, but please fix it :)

> +.IP
>  The
>  .B AT_NO_AUTOMOUNT
> -flag has no effect if the mount point has already been mounted over.
> +flag can be used in tools that scan directories
> +to prevent mass-automounting of a directory of automount points.
> +.IP
> +All of
> +.BR stat () ,

s/() ,/(2),/

> +.BR lstat () ,

s/() ,/(2),/

> +and
> +.BR fstatat ()

s/()/(2)/

> +act as though
> +.B AT_NO_AUTOMOUNT
> +was set.
> +.IP
>  This flag is Linux-specific; define
>  .B _GNU_SOURCE
>  .\" Before glibc 2.16, defining _ATFILE_SOURCE sufficed
> -- 
> 2.31.1
> 


-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/