| Message ID | 81cd5da550e06de8e85dcadef4909ff5f1d23319.1601992016.git.mchehab+huawei@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | None | expand |
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? > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.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? > 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. Also please use textwidth=70 for consistency with the rest of the file. - Eric
Em Tue, 6 Oct 2020 12:19:53 -0700 Eric Biggers <ebiggers@kernel.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@kernel.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
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 ``<linux/fscrypt.h>`` 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 follows:: struct fscrypt_get_key_status_arg { /* input */ @@ -1148,12 +1143,12 @@ Implementation details Encryption context ------------------ -An encryption policy is represented on-disk by a :c:type:`struct -fscrypt_context_v1` or a struct fscrypt_context_v2. It is -up to individual filesystems to decide where to store it, but normally -it would be stored in a hidden extended attribute. It should *not* be -exposed by the xattr-related system calls such as getxattr() and -setxattr() because of the special semantics of the encryption xattr. +An encryption policy is represented on-disk by struct fscrypt_context_v1 +or struct fscrypt_context_v2. It is up to individual filesystems to decide +where to store it, but normally it would be stored in a hidden extended +attribute. It should *not* be exposed by the xattr-related system calls +such as getxattr() and setxattr() because of the special semantics of the +encryption xattr. (In particular, there would be much confusion if an encryption policy were to be added to or removed from anything other than an empty directory.) These structs are defined as follows:: @@ -1249,8 +1244,8 @@ a strong "hash" of the ciphertext filename, along with the optional filesystem-specific hash(es) needed for directory lookups. This allows the filesystem to still, with a high degree of confidence, map the filename given in ->lookup() back to a particular directory entry -that was previously listed by readdir(). See :c:type:`struct -fscrypt_nokey_name` in the source for more details. +that was previously listed by readdir(). See struct fscrypt_nokey_name +in the source for more details. Note that the precise way that filenames are presented to userspace without the key is subject to change in the future. It is only meant
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 <mchehab+huawei@kernel.org> --- Documentation/filesystems/fscrypt.rst | 51 ++++++++++++--------------- 1 file changed, 23 insertions(+), 28 deletions(-)