linux-kernel - Re: [PATCH] fs: doc: describe 'pinned' parameter in do_lock

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] [day] [month] [year] [list]

Message-ID: <20250924212207.GV39973@ZenIV>
Date: Wed, 24 Sep 2025 22:22:07 +0100
From: Al Viro <viro@...iv.linux.org.uk>
To: Kriish Sharma <kriish.sharma2006@...il.com>
Cc: brauner@...nel.org, jack@...e.cz, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, david.hunter.linux@...il.com,
	skhan@...uxfoundation.org
Subject: Re: [PATCH] fs: doc: describe 'pinned' parameter in do_lock_mount()

On Wed, Sep 24, 2025 at 09:57:30PM +0100, Al Viro wrote:
> On Wed, Sep 24, 2025 at 07:36:11PM +0000, Kriish Sharma wrote:
> > The kernel-doc comment for do_lock_mount() was missing a description
> > for the 'pinned' parameter:
> > 
> >   Warning: fs/namespace.c:2772 function parameter 'pinned' not described
> >   in 'do_lock_mount'
> > 
> > This patch adds a short description:
> > 
> >   @pinned: receives the pinned mountpoint
> > 
> > to fix the warning and improve documentation clarity.
> 
> Sigh...  There we go again...
> 	1.  It does not improve documentation clarity - it adds
> a misleading line to an opaque chunk of text that does not match
> what the function *does*.
> 	2.  In -next both the calling conventions and the comment
> are both changed, hopefully making it more readable.
> 	3.  Essentially the same patch has already been posted and
> discussed.
> 
> NAKed-by: Al Viro <viro@...iv.linux.org.uk>

PS: as for the clarity, I'd like to point out that with your patch
applied nothing in the comments explains what is *done* to that
argument - it's not even mentioned there.  Incidentally, "receives"
normally implies an IN argument; this is an OUT one (function set the
environment for mounting something at given location and stores the
resulting context in caller-supplied structure).

Comment quality needs to be improved and these warnings actually
do catch some of the stale (and generally incomprehensible) ones.
Unfortunately, it's easy to fool the heuristics without doing
anything about the underlying problem, in effect hiding it.

Folks, please don't do that - this is not an improvement.  If you
spot something of that sort and want to take a pass at fixing it,
more power to you, but the main criteria here would be "does that
text make it easier to understand the function in question and/or
the rules for using it".  If it's genuinely hard to figure out,
don't hesitate to ask.