linux-kernel - Re: [PATCH v2 03/11] fsopen.2: document 'new' mount api

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]

Message-ID: <3u44qtys5a6k7hspgs6syhhuuicwjtdzdsdhe65j3kilacmfuj@pqsaofcc4lul>
Date: Thu, 7 Aug 2025 15:42:39 +0200
From: Alejandro Colomar <alx@...nel.org>
To: Aleksa Sarai <cyphar@...har.com>
Cc: "Michael T. Kerrisk" <mtk.manpages@...il.com>, 
	Alexander Viro <viro@...iv.linux.org.uk>, Jan Kara <jack@...e.cz>, Askar Safin <safinaskar@...omail.com>, 
	"G. Branden Robinson" <g.branden.robinson@...il.com>, linux-man@...r.kernel.org, linux-api@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	David Howells <dhowells@...hat.com>, Christian Brauner <brauner@...nel.org>
Subject: Re: [PATCH v2 03/11] fsopen.2: document 'new' mount api

Hi Aleksa,

On Thu, Aug 07, 2025 at 10:50:17PM +1000, Aleksa Sarai wrote:
> > > +A filesystem configuration context is an in-kernel representation of a pending
> > > +transaction,
> > 
> > This page still needs semantic newlines.  (Please review all pages
> > regarding that.)  (In this specific sentence, I'd break after 'is'.)
> 
> I did try adding them to this page (and all of the other pages -- I
> suspect the pages later in the patchset have more aggressive newlining).
> If you compare the newline placement between v1 and v2 you'll see that I
> have added a lot of newlines in all of the man-pages, but it's possible
> I missed a couple of sentences like this one.

Yup, it's quite better.  Thanks!

> To be honest I feel quite lost where the "semantic newlines" school
> would deem appropriate to place newlines, and man-pages(7) is very terse
> on the topic. Outside of very obvious examples,
> it just feels wrong
> to have such choppy
> line break usage.
> I understand
> the argument that
> this helps
> with reviewing diffs,
> but I really find it
> incredibly unnatural.
> (And this tongue-in-cheek example
> is probably wrong too.)

I understand.  The guidelines I use are:

	If there's punctuation, break.
	If there isn't punctuation, but the sentence would go past the
	80-char right margin, try to find the best point to break (this
	is sometimes hard or subjective).
	Other than that, there's no need to break.

Does that seem reasonable?  (I can always amend a few cases that you
don't know where to split.)

> 
> > > +containing a set of configuration parameters that are to be applied
> > > +when creating a new instance of a filesystem
> > > +(or modifying the configuration of an existing filesystem instance,
> > > +such as when using
> > > +.BR fspick (2)).
> > > +.P
> > > +After obtaining a filesystem configuration context with
> > > +.BR fsopen (),
> > > +the general workflow for operating on the context looks like the following:
> > > +.IP (1) 5
> > > +Pass the filesystem context file descriptor to
> > > +.BR fsconfig (2)
> > > +to specify any desired filesystem parameters.
> > > +This may be done as many times as necessary.
> > > +.IP (2)
> > > +Pass the same filesystem context file descriptor to
> > 
> > Do we need to say "same"?  I guess it's obvious.  Or do you expect
> > any confusion if we don't?
> 
> The first time I saw this interface I was confused when you pass
> which file descriptor (especially around the FSCONFIG_CMD_CREATE stage),
> so I felt it better to make it clear which file descriptor we are
> talking about.

Okay.

> > > +.EX
> > > +int fsfd, mntfd;
> > > +\&
> > > +fsfd = fsopen("ext4", FSOPEN_CLOEXEC);
> > > +fsconfig(fsfd, FSCONFIG_SET_FLAG, "ro", NULL, 0);
> > > +fsconfig(fsfd, FSCONFIG_SET_PATH, "source", "/dev/sdb1", AT_FDCWD);
> > > +fsconfig(fsfd, FSCONFIG_SET_FLAG, "noatime", NULL, 0);
> > > +fsconfig(fsfd, FSCONFIG_SET_FLAG, "acl", NULL, 0);
> > > +fsconfig(fsfd, FSCONFIG_SET_FLAG, "user_xattr", NULL, 0);
> > > +fsconfig(fsfd, FSCONFIG_SET_FLAG, "iversion", NULL, 0)
> > > +fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0);
> > > +mntfd = fsmount(fsfd, FSMOUNT_CLOEXEC, MOUNT_ATTR_RELATIME);
> > > +move_mount(mntfd, "", AT_FDCWD, "/mnt", MOVE_MOUNT_F_EMPTY_PATH);
> > > +.EE
> > > +.in
> > > +.P
> > > +First, an ext4 configuration context is created and attached to the file
> > 
> > Here, I'd break after the ',', and if you need to break again, after
> > 'created'.
> 
> Okay, I wanted to avoid having lines with single words due to semantic
> newlines, but if that's what you prefer I can update that everywhere...

I don't have a strong opinion on that.  I sometimes avoid the break if
the rest of the sentence is short and all fits in one line, but if you
already need to break, that'd be the first obvious place to look at.
Other times, I have a more pedantic day, and split at every comma, even
unnecessarily.


Cheers,
Alex

-- 
<https://www.alejandro-colomar.es/>

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)