unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: "G. Branden Robinson via Libc-alpha" <libc-alpha@sourceware.org>
To: "Alejandro Colomar (man-pages)" <alx.manpages@gmail.com>
Cc: linux-man@vger.kernel.org, Christian Brauner <brauner@kernel.org>,
	Glibc <libc-alpha@sourceware.org>,
	Christoph Hellwig <hch@infradead.org>,
	Michael Kerrisk <mtk.manpages@gmail.com>,
	linux-fsdevel@vger.kernel.org,
	Christian Brauner <christian.brauner@ubuntu.com>
Subject: Re: [PATCH v2] mount_setattr.2: New manual page documenting the mount_setattr() system call
Date: Sat, 31 Jul 2021 11:42:52 +1000	[thread overview]
Message-ID: <20210731014251.whqfubv3hzu3mssw@localhost.localdomain> (raw)
In-Reply-To: <9ba8d98e-dee9-1d8d-0777-bb5496103e24@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 7625 bytes --]

Hi, Alex!

At 2021-07-30T20:15:43+0200, Alejandro Colomar (man-pages) wrote:
> > +With each extension that changes the size of
> > +.I struct mount_attr
> > +the kernel will expose a define of the form
> > +.B MOUNT_ATTR_SIZE_VER<number> .
> 
> s/.B/.BR/

I would say this is, properly considered, an instance of the notorious
three-font problem.  "number" should be in italics, and the angle
brackets are certainly neither literal nor variable, but are injected to
get around the most dreaded markup problem in man(7).

But it need not be dreaded anymore.

There are two ways to address this; both require *roff escape sequences.

My preference is to use \c, the output line continuation escape
sequence.  (Ingo Schwarze, OpenBSD committer and mandoc(1) maintainer,
has the opposite preference, for \f escapes).

I recommend:

.BI MOUNT_ATTR_SIZE_VER number\c
\&.

(The non-printing input break, \& can be seen in several contexts; here,
it prevents the period from being interpreted specially the beginning of
the line, marking it a "control line" like the macro call immediately
previous.)

The period will be set in the roman style.  This problem and its
workarounds are documented in groff_man_style(7)[1].

> > +The effect of this change will be a mount or mount tree that is read-only,
> > +blocks the execution of setuid binaries but does allow to execute programs and
> 
> I'm not sure about this:
> 
> I've checked other pages that mention setuid, and I've seen different
> things.
> But none of them use setuid as an English word.
> Maybe have a look at similar pages and try to use a similar syntax.
> 
> grep -rni setuid man?

It's common in technical literature to introduce specialized terminology
and subsequently apply it attributively.

Personally, I would style "setuid" in italics in the above example; that
is how I would expect to see it in a printed manual.

Even more explicitly, one could write

	execution of
	.IR setuid -using
	binaries,

While I'm here I will note that there should be a comma as noted above,
and the seemingly ineradicable Denglish construction of following the
verb "allow" with an infinitive should be recast.

My suggestion:

	but allows execution of programs and access to device nodes.

> > +access to devices nodes.
> > +In the new mount api the access time values are an enum starting from 0.
> > +Even though they are an enum in contrast to the other mount flags such as
> > +.BR MOUNT_ATTR_NOEXEC
> 
> s/.BR/.B/

Alex (and others), if you have access to groff from its Git HEAD, you
might be interested in trying my experimental CHECKSTYLE feature.  You
use it by setting a register by that name when calling groff.  Roughly
speaking, increasing the value turns up the linting.

groff -man -rCHECKSTYLE=n

where n is:
	1	Emits a warning if the argument count is wrong to TH or
		the six font style alternation macros.
	2	As 1, plus it complains of usage of deprecated macros
		(AT, UC, DT, PD).  And some day I'll be adding OP to
		that list.
	3	As 2, plus it complains of blank lines or lines with
		leading spaces.

Setting 1 has saved me from goofs prior to committing man page changes
many times already in its short life.  Setting 2 reminds me every day
that I need to fix up groff(7).  I don't usually provoke setting 3, but
it has proven its worth at least once.

The above is the most documentation this feature has yet seen, and I'd
appreciate feedback.

> > +.IP \(bu
> > +The mount must be a detached/anonymous mount,
> > +i.e. it must have been created by calling

"i.e." should be followed by a comma, just like "e.g.", as they
substitute for the English phrases "that is" and "for example",
respectively.

And, yes, I'd semantically line break after that comma, too.  ;-)

> > +.BR open_tree (2)
> > +with the
> > +.I OPEN_TREE_CLONE
> > +flag and it must not already have been visible in the filesystem.
> > +.RE
> > +.IP
> 
> .IP here doesn't mean anything, if I'm not mistaken.

It certainly _should_--it should cause the insertion of vertical space.
(It would also cause a break, but .RE already did that.)

The interaction of .IP and .RS/.RE is tricky and can probably be blamed,
back in 2017, for irritating me to the point that I became a groff
developer.  I've documented it as extensively as I am able in
groff_man_style(7)[1].

> We don't want to format the ending period, as discussed in the linked
> thread.  Consider using .IR as explained there.  Maybe nonbreaking spaces at
> some points of this sequence are also necessary.
> 
> Nonbreaking spaces are \~
> 
> You can see a discussion about nonbreaking spaces (of yesterday) here:
> <https://lore.kernel.org/linux-man/90d6fcc8-70e6-5c44-5703-1c2bf2ad6913@gmail.com/T/#u>

Thanks for reminding me--I need to get back to you with a suggested
patch.  I am so bad at preparing patches for the man-pages project.  :(

> > +.RE
> > +.RE
> > +.IP
> 
> Another unused .IP?
> 
> What did you mean?

Here's another point to consider.  Maybe he wants to preserve the
indentation amount "cached" by the last IP macro with such an argument.

   Horizontal and vertical spacing
       The indent argument accepted by .RS, .IP, .TP, and the deprecated
       .HP  is a number plus an optional scaling indicator.  If no scal‐
       ing indicator is given, the man package assumes “n”.  An indenta‐
       tion  specified in a call to .IP, .TP, or the deprecated .HP per‐
       sists until (1) another of these macros is  called  with  an  ex‐
       plicit indent argument, or (2) .SH, .SS, or .P or its synonyms is
       called; these clear the indentation  entirely.   Relative  insets
       created  by  .RS move the left margin and persist until .RS, .RE,
       .SH, or .SS is called.

I realize the above text is pretty dense.  These matters were almost
undocumented in groff 1.22.3 and for many years before that.

> > +                exit_log("%m - Failed top open %s\n", optarg);
> 
> 
> Use \e to write the escape sequence in groff.  See groff_man(7):
> 
>        \e     Widely used in man pages to  represent  a  backslash
>               output  glyph.  It works reliably as long as the .ec
>               request is not used, which should  never  happen  in
>               man pages, and it is slightly more portable than the
>               more exact ‘\(rs’  (“reverse  solidus”)  escape  se‐
>               quence.
> 
> \n is interpreted by groff(1) and not translated verbatim into the
> output.

Yes.  It interpolates a register named '"', in this case (which is a
valid roff identifier).  That's probably not defined, so groff will
define it automatically, assign zero to it, and then interpolated it. So
you'd end up with %s0.

I think it bears restating that code examples in man pages, whether set
with the .EX/.EE macros or otherwise, are not "literal" or
"preformatted" regions[1].  .EX does two things only: it disables
filling (which also necessarily disables automatic breaking, automatic
hyphenation, and adjustment), and it changes the font family for
typesetter devices (to Courier).  The escape character remains a
backslash and it remains fully armed and operational, as Emperor
Palpatine might say.

Regards,
Branden

[1] https://www.man7.org/linux/man-pages/man7/groff_man_style.7.html
[2] Long ago, in the 1970s, there was an ".li" (for "literal") request
    in Unix troff.  It was taken out even before Version 7 Unix was
    published.)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2021-07-31  1:43 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20210730094121.3252024-1-brauner@kernel.org>
2021-07-30 18:15 ` [PATCH v2] mount_setattr.2: New manual page documenting the mount_setattr() system call Alejandro Colomar (man-pages) via Libc-alpha
2021-07-31  1:42   ` G. Branden Robinson via Libc-alpha [this message]
2021-07-31 11:20     ` Alejandro Colomar (man-pages) via Libc-alpha
2021-07-31  9:43   ` Christian Brauner
2021-07-31 12:30     ` Alejandro Colomar (man-pages) via Libc-alpha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210731014251.whqfubv3hzu3mssw@localhost.localdomain \
    --to=libc-alpha@sourceware.org \
    --cc=alx.manpages@gmail.com \
    --cc=brauner@kernel.org \
    --cc=christian.brauner@ubuntu.com \
    --cc=g.branden.robinson@gmail.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-man@vger.kernel.org \
    --cc=mtk.manpages@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).