linux-kernel - Re: [PATCH v5 35/52] docs: fs: fscrypt.rst: get rid of :c:type: tags

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]

Date:   Wed, 14 Oct 2020 09:12:40 +0200
From:   Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To:     Eric Biggers <ebiggers@...nel.org>
Cc:     Linux Doc Mailing List <linux-doc@...r.kernel.org>,
        Jonathan Corbet <corbet@....net>,
        "Theodore Y. Ts'o" <tytso@....edu>,
        Jaegeuk Kim <jaegeuk@...nel.org>,
        linux-fscrypt@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 35/52] docs: fs: fscrypt.rst: get rid of :c:type:
 tags

Em Tue, 6 Oct 2020 12:19:53 -0700
Eric Biggers <ebiggers@...nel.org> escreveu:

> On Tue, Oct 06, 2020 at 04:03:32PM +0200, Mauro Carvalho Chehab wrote:
> > The :c:type: tag has problems with Sphinx 3.x, as structs
> > there should be declared with c:struct.
> > 
> > So, remove them, relying at automarkup.py extension to
> > convert them into cross-references.  
> 
> I tried 'make htmldocs' before and after your patchset ("sphinx3-fixes-v5").
> Before, all the struct fscrypt_* are rendered in code font.  After, they are
> rendered in the regular text font.  Is that really working as intended?

It is up to automarkup.py to change from "struct foo" into:
	:c:type:`struct foo` (Sphinx 2.x)
or:
	:c:struct:`foo` (Sphinx 3.x)

At v5, the automarkup.py extension was disabled, as it was broken
with Sphinx > 2.x. At v6, I added a patch from Nícolas addressing
it.

It should be said that, currently, if there's no documentation for 
"foo", automarkup will just keep using the regular text font,
keeping the text untouched.

> 
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
> > ---
> >  Documentation/filesystems/fscrypt.rst | 51 ++++++++++++---------------
> >  1 file changed, 23 insertions(+), 28 deletions(-)
> >   
> 
> Why are the changes to fscrypt.rst split between two patches,
> 
> 	docs: get rid of :c:type explicit declarations for structs
> 
> and
> 
> 	docs: fs: fscrypt.rst: get rid of :c:type: tags
> 
> ?  They're the same type of changes.  The first just removes half the :c:type:
> tags, and the second removes the rest.  Shouldn't it be one patch?
> 

The reason is just because it was easier this way. 

On the first patch, I used sed to replace structs on a 
semi-automated way, checking the results.

at the second one, I addressed the remaining symbols manually.

Anyway, at the new version, I just placed everything related
to fscript.rst at the same patch, to make easier to review.

> > diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
> > index 4f858b38a412..46a9d1bd2ab5 100644
> > --- a/Documentation/filesystems/fscrypt.rst
> > +++ b/Documentation/filesystems/fscrypt.rst
> > @@ -437,8 +437,7 @@ FS_IOC_SET_ENCRYPTION_POLICY
> >  The FS_IOC_SET_ENCRYPTION_POLICY ioctl sets an encryption policy on an
> >  empty directory or verifies that a directory or regular file already
> >  has the specified encryption policy.  It takes in a pointer to a
> > -struct fscrypt_policy_v1 or a :c:type:`struct
> > -fscrypt_policy_v2`, defined as follows::
> > +struct fscrypt_policy_v1 or a struct fscrypt_policy_v2, defined as follows::  
> [...]
> >  If the file is not yet encrypted, then FS_IOC_SET_ENCRYPTION_POLICY
> >  verifies that the file is an empty directory.  If so, the specified
> > @@ -637,9 +634,8 @@ The FS_IOC_GET_ENCRYPTION_POLICY ioctl can also retrieve the
> >  encryption policy, if any, for a directory or regular file.  However,
> >  unlike `FS_IOC_GET_ENCRYPTION_POLICY_EX`_,
> >  FS_IOC_GET_ENCRYPTION_POLICY only supports the original policy
> > -version.  It takes in a pointer directly to a :c:type:`struct
> > -fscrypt_policy_v1` rather than a :c:type:`struct
> > -fscrypt_get_policy_ex_arg`.
> > +version.  It takes in a pointer directly to struct fscrypt_policy_v1
> > +rather than struct fscrypt_get_policy_ex_arg.  
> 
> In some cases you deleted the "a" in "a struct" but in other cases you didn't.
> Intentional?  It seems the file should consistently use one style or the other.

Yes, it was intentional. On almost all other docs documents I reviewed or
converted, they say "struct" instead of "a struct".

At the second version, I did the replacement on a consistent way.

> 
> Also please use textwidth=70 for consistency with the rest of the file.

Done. At the new patch I posted, none of the lines touched by the
patch uses more than 70 columns.

You may notice that I opted to keep "struct foo" at the same line.
This is not a mandatory requirement for automarkup.py to work, but
I would recommend keeping them at the same line, as, if someone tries to
do something like:

	$ git grep "struct foo" Documentation/

It would be able to find them.


Thanks,
Mauro