Re: [PATCH v6 35/80] docs: fs: fscrypt.rst: get rid of :c:type: tags

2020-10-15 Thread Jonathan Corbet
On Thu, 15 Oct 2020 07:32:07 +0200
Mauro Carvalho Chehab  wrote:

> > That will apply to most (maybe all) of the structures mentioned in this 
> > file.
> > I expected that if the documentation system now automatically recognizes
> > 'struct foo', then it would render it in code font even when 'struct foo' 
> > isn't
> > documented.  Any particular reason why that isn't the case?  Not like I care
> > much myself, but it's a bit unexpected and it means this change actually 
> > makes
> > the rendered documentation look worse...  
> 
> Yeah, I agree that using monospaced fonts on this case too would
> be nice. The C domain actually uses italic monospaced fonts for
> broken XREFs.
> 
> I suspect that changing this at automarkup.py would be simple, but
> not sure if it would be safe.
> 
> Jon can tell more about that, as he's the author of automarkup,
> but I suspect that the reason for the current behavior is to avoid 
> false-positives. 

Automarkup has always behaved that way because ... well, because nobody
got around to changing it.  I don't see any reason not to use a monospace
font for such things, just without a link; shouldn't be a problem to do.
I'll see if I can't get to it once things stabilize a bit.

Thanks,

jon


Re: [PATCH v6 35/80] docs: fs: fscrypt.rst: get rid of :c:type: tags

2020-10-15 Thread Mauro Carvalho Chehab
Em Thu, 15 Oct 2020 09:36:05 -0700
Eric Biggers  escreveu:

> On Thu, Oct 15, 2020 at 07:32:07AM +0200, Mauro Carvalho Chehab wrote:
> > On the other hand, if one finds a valid "struct foo" using normal
> > fonts, this would mean that either the doc is outdated, mentioning
> > an struct that were removed/renamed or that there's a missing 
> > kernel-doc markup.
> > 
> > In any case, the fix is to simply fix the kernel-doc markup for
> > struct foo.
> > 
> > I guess in the future automarkup.py could issue a warning in
> > order to warn about missing cross-references, perhaps when
> > W=1 or W=2 is used.  
> 
> Well, most structs that fscrypt.rst refers to are defined in
> include/uapi/linux/fscrypt.h.  The whole fscrypt UAPI, including the fields of
> these structs, is documented in fscrypt.rst.  So I didn't really intend the
> fscrypt UAPI structs to have kerneldoc comments, as people are supposed to 
> refer
> to the documentation in fscrypt.rst instead.  We could have both, but it 
> feels a
> bit redundant.

Yeah, we do the same on V4L: the uAPI doesn't use kernel-docs. It is
documented, instead, at ReST files.

In any case, if all structs are documented, automarkup should
be using monospaced fonts and be generating cross-references.

If not, the regular expressions there may need tweaks ;-)

Thanks,
Mauro


Re: [PATCH v6 35/80] docs: fs: fscrypt.rst: get rid of :c:type: tags

2020-10-15 Thread Eric Biggers
On Thu, Oct 15, 2020 at 07:32:07AM +0200, Mauro Carvalho Chehab wrote:
> On the other hand, if one finds a valid "struct foo" using normal
> fonts, this would mean that either the doc is outdated, mentioning
> an struct that were removed/renamed or that there's a missing 
> kernel-doc markup.
> 
> In any case, the fix is to simply fix the kernel-doc markup for
> struct foo.
> 
> I guess in the future automarkup.py could issue a warning in
> order to warn about missing cross-references, perhaps when
> W=1 or W=2 is used.

Well, most structs that fscrypt.rst refers to are defined in
include/uapi/linux/fscrypt.h.  The whole fscrypt UAPI, including the fields of
these structs, is documented in fscrypt.rst.  So I didn't really intend the
fscrypt UAPI structs to have kerneldoc comments, as people are supposed to refer
to the documentation in fscrypt.rst instead.  We could have both, but it feels a
bit redundant.

- Eric


Re: [PATCH v6 35/80] docs: fs: fscrypt.rst: get rid of :c:type: tags

2020-10-14 Thread Mauro Carvalho Chehab
Em Wed, 14 Oct 2020 14:59:54 -0700
Eric Biggers  escreveu:

> On Wed, Oct 14, 2020 at 08:59:07AM +0200, Mauro Carvalho Chehab wrote:
> > [PATCH v6.1 35/80] docs: fs: fscrypt.rst: get rid of :c:type: tags
> > 
> > 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.
> >
> > Signed-off-by: Mauro Carvalho Chehab   
> 
> "relying at" => "relying on".
> 
> Otherwise looks fine, you can add:
> 
> Reviewed-by: Eric Biggers 

Thank you for reviewing it!

> I do still wonder about your comment though:
> 
> > 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.  
> 
> That will apply to most (maybe all) of the structures mentioned in this file.
> I expected that if the documentation system now automatically recognizes
> 'struct foo', then it would render it in code font even when 'struct foo' 
> isn't
> documented.  Any particular reason why that isn't the case?  Not like I care
> much myself, but it's a bit unexpected and it means this change actually makes
> the rendered documentation look worse...

Yeah, I agree that using monospaced fonts on this case too would
be nice. The C domain actually uses italic monospaced fonts for
broken XREFs.

I suspect that changing this at automarkup.py would be simple, but
not sure if it would be safe.

Jon can tell more about that, as he's the author of automarkup,
but I suspect that the reason for the current behavior is to avoid 
false-positives. 

I mean, if "struct foo" symbol doesn't exist at the C domain, this
might mean that the parser is doing something wrong. So, a more
conservative approach is to keep the string as-is.

On the other hand, if one finds a valid "struct foo" using normal
fonts, this would mean that either the doc is outdated, mentioning
an struct that were removed/renamed or that there's a missing 
kernel-doc markup.

In any case, the fix is to simply fix the kernel-doc markup for
struct foo.

I guess in the future automarkup.py could issue a warning in
order to warn about missing cross-references, perhaps when
W=1 or W=2 is used.

Thanks,
Mauro


Re: [PATCH v6 35/80] docs: fs: fscrypt.rst: get rid of :c:type: tags

2020-10-14 Thread Eric Biggers
On Wed, Oct 14, 2020 at 08:59:07AM +0200, Mauro Carvalho Chehab wrote:
> [PATCH v6.1 35/80] docs: fs: fscrypt.rst: get rid of :c:type: tags
> 
> 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.
>
> Signed-off-by: Mauro Carvalho Chehab 

"relying at" => "relying on".

Otherwise looks fine, you can add:

Reviewed-by: Eric Biggers 

I do still wonder about your comment though:

> 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.

That will apply to most (maybe all) of the structures mentioned in this file.
I expected that if the documentation system now automatically recognizes
'struct foo', then it would render it in code font even when 'struct foo' isn't
documented.  Any particular reason why that isn't the case?  Not like I care
much myself, but it's a bit unexpected and it means this change actually makes
the rendered documentation look worse...

- Eric


Re: [PATCH v6 35/80] docs: fs: fscrypt.rst: get rid of :c:type: tags

2020-10-14 Thread Mauro Carvalho Chehab
Em Tue, 13 Oct 2020 10:25:12 -0700
Eric Biggers  escreveu:

> On Tue, Oct 13, 2020 at 01:53:50PM +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.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> 
> I left some comments on v5 which weren't addressed.
> 
> - Eric

Sorry, I missed your reply. I'm dropping fscrypt.rst changes
that were on this patch:

[PATCH v6 23/80] docs: get rid of :c:type explicit declarations for 
structs

and replacing with the enclosed one that should be addressing
your issues.

Thanks,
Mauro

[PATCH v6.1 35/80] docs: fs: fscrypt.rst: get rid of :c:type: tags

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.

Signed-off-by: Mauro Carvalho Chehab 

diff --git a/Documentation/filesystems/fscrypt.rst 
b/Documentation/filesystems/fscrypt.rst
index 423c5a0daf45..44b67ebd6e40 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -436,9 +436,9 @@ 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
-:c:type:`struct fscrypt_policy_v1` or a :c:type:`struct
-fscrypt_policy_v2`, defined as follows::
+has the specified encryption policy.  It takes in a pointer to
+struct fscrypt_policy_v1 or struct fscrypt_policy_v2, defined as
+follows::
 
 #define FSCRYPT_POLICY_V1   0
 #define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
@@ -464,11 +464,11 @@ fscrypt_policy_v2`, defined as follows::
 
 This structure must be initialized as follows:
 
-- ``version`` must be FSCRYPT_POLICY_V1 (0) if the struct is
-  :c:type:`fscrypt_policy_v1` or FSCRYPT_POLICY_V2 (2) if the struct
-  is :c:type:`fscrypt_policy_v2`.  (Note: we refer to the original
-  policy version as "v1", though its version code is really 0.)  For
-  new encrypted directories, use v2 policies.
+- ``version`` must be FSCRYPT_POLICY_V1 (0) if
+  struct fscrypt_policy_v1 is used or FSCRYPT_POLICY_V2 (2) if
+  struct fscrypt_policy_v2 is used. (Note: we refer to the original
+  policy version as "v1", though its version code is really 0.)
+  For new encrypted directories, use v2 policies.
 
 - ``contents_encryption_mode`` and ``filenames_encryption_mode`` must
   be set to constants from  which identify the
@@ -508,9 +508,9 @@ This structure must be initialized as follows:
   replaced with ``master_key_identifier``, which is longer and cannot
   be arbitrarily chosen.  Instead, the key must first be added using
   `FS_IOC_ADD_ENCRYPTION_KEY`_.  Then, the ``key_spec.u.identifier``
-  the kernel returned in the :c:type:`struct fscrypt_add_key_arg` must
-  be used as the ``master_key_identifier`` in the :c:type:`struct
-  fscrypt_policy_v2`.
+  the kernel returned in the struct fscrypt_add_key_arg must
+  be used as the ``master_key_identifier`` in
+  struct fscrypt_policy_v2.
 
 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
@@ -590,7 +590,7 @@ FS_IOC_GET_ENCRYPTION_POLICY_EX
 The FS_IOC_GET_ENCRYPTION_POLICY_EX ioctl retrieves the encryption
 policy, if any, for a directory or regular file.  No additional
 permissions are required beyond the ability to open the file.  It
-takes in a pointer to a :c:type:`struct fscrypt_get_policy_ex_arg`,
+takes in a pointer to struct fscrypt_get_policy_ex_arg,
 defined as follows::
 
 struct fscrypt_get_policy_ex_arg {
@@ -637,9 +637,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.
 
 The error codes for FS_IOC_GET_ENCRYPTION_POLICY are the same as those
 for FS_IOC_GET_ENCRYPTION_POLICY_EX, except that
@@ -680,8 +679,7 @@ the filesystem, making all files on the filesystem which 
were
 encrypted using that key appear "unlocked", i.e. in plaintext form.
 It can be executed on any file or directory on the target filesystem,
 but using the filesystem's root directory is recommended.  It takes in
-a pointer to a :c:type:`struct fscrypt_add_key_arg`, defined as
-follows::
+a pointer to struct fscrypt_add_key_arg, defined as follows::
 
 struct fscrypt_add_key_arg {
 struct 

Re: [PATCH v6 35/80] docs: fs: fscrypt.rst: get rid of :c:type: tags

2020-10-13 Thread Eric Biggers
On Tue, Oct 13, 2020 at 01:53:50PM +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.
> 
> Signed-off-by: Mauro Carvalho Chehab 

I left some comments on v5 which weren't addressed.

- Eric


[PATCH v6 35/80] docs: fs: fscrypt.rst: get rid of :c:type: tags

2020-10-13 Thread Mauro Carvalho Chehab
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.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/filesystems/fscrypt.rst | 51 ---
 1 file changed, 23 insertions(+), 28 deletions(-)

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::
 
 #define FSCRYPT_POLICY_V1   0
 #define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
@@ -464,11 +463,10 @@ fscrypt_policy_v2`, defined as follows::
 
 This structure must be initialized as follows:
 
-- ``version`` must be FSCRYPT_POLICY_V1 (0) if the struct is
-  :c:type:`fscrypt_policy_v1` or FSCRYPT_POLICY_V2 (2) if the struct
-  is :c:type:`fscrypt_policy_v2`.  (Note: we refer to the original
-  policy version as "v1", though its version code is really 0.)  For
-  new encrypted directories, use v2 policies.
+- ``version`` must be FSCRYPT_POLICY_V1 (0) if struct fscrypt_policy_v1
+  is used or FSCRYPT_POLICY_V2 (2) if struct fscrypt_policy_v2 is used.
+  (Note: we refer to the original policy version as "v1", though its
+  version code is really 0.)  For new encrypted directories, use v2 policies.
 
 - ``contents_encryption_mode`` and ``filenames_encryption_mode`` must
   be set to constants from  which identify the
@@ -509,8 +507,7 @@ This structure must be initialized as follows:
   be arbitrarily chosen.  Instead, the key must first be added using
   `FS_IOC_ADD_ENCRYPTION_KEY`_.  Then, the ``key_spec.u.identifier``
   the kernel returned in the struct fscrypt_add_key_arg must
-  be used as the ``master_key_identifier`` in the :c:type:`struct
-  fscrypt_policy_v2`.
+  be used as the ``master_key_identifier`` in struct fscrypt_policy_v2.
 
 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.
 
 The error codes for FS_IOC_GET_ENCRYPTION_POLICY are the same as those
 for FS_IOC_GET_ENCRYPTION_POLICY_EX, except that
@@ -717,10 +713,9 @@ as follows:
   ``key_spec.type`` must contain FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR, and
   ``key_spec.u.descriptor`` must contain the descriptor of the key
   being added, corresponding to the value in the
-  ``master_key_descriptor`` field of :c:type:`struct
-  fscrypt_policy_v1`.  To add this type of key, the calling process
-  must have the CAP_SYS_ADMIN capability in the initial user
-  namespace.
+  ``master_key_descriptor`` field of struct fscrypt_policy_v1.
+  To add this type of key, the calling process must have the
+  CAP_SYS_ADMIN capability in the initial user namespace.
 
   Alternatively, if the key is being added for use by v2 encryption
   policies, then ``key_spec.type`` must contain
@@ -737,8 +732,8 @@ as follows:
 
 - ``key_id`` is 0 if the raw key is given directly in the ``raw``
   field.  Otherwise ``key_id`` is the ID of a Linux keyring key of
-  type "fscrypt-provisioning" whose payload is a :c:type:`struct
-  fscrypt_provisioning_key_payload` whose ``raw`` field contains the
+  type "fscrypt-provisioning" whose payload is
+  struct fscrypt_provisioning_key_payload whose ``raw`` field contains the
   raw key and whose ``type`` field matches ``key_spec.type``.  Since
   ``raw`` is variable-length, the total size of this key's payload
   must be ``sizeof(struct fscrypt_provisioning_key_payload)`` plus the
@@ -956,8 +951,8 @@ FS_IOC_GET_ENCRYPTION_KEY_STATUS
 The FS_IOC_GET_ENCRYPTION_KEY_STATUS ioctl retrieves the status of a
 master encryption key.  It can be executed on any file or directory on
 the target filesystem, but using the filesystem's root directory is
-recommended.  It takes in a pointer to a :c:type:`struct
-fscrypt_get_key_status_arg`, defined as follows::
+recommended.  It takes in a pointer to struct fscrypt_get_key_status_arg,
+defined as