Re: [PATCH v1 3/4] landlock: Improve kernel-doc "Return:" section consistency

["Günther Noack" <gnoack3000@gmail.com>]

Just a few comment phrasing nits below

On Wed, Mar 04, 2026 at 08:31:26PM +0100, Mickaël Salaün wrote:
> The canonical kernel-doc form is "Return:" (singular, without trailing
> "s").  Normalize all existing "Returns:" occurrences across the Landlock
> source tree to the canonical form.
> 
> Also fix capitalization for consistency.  Balance descriptions to
> describe all possible returned values.
> 
> Consolidate bullet-point return descriptions into inline text for
> functions with simple two-value or three-value returns for consistency.
> 
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  security/landlock/cred.h    |  2 +-
>  security/landlock/domain.c  |  4 ++--
>  security/landlock/fs.c      | 26 +++++++++++---------------
>  security/landlock/id.c      |  2 +-
>  security/landlock/ruleset.c |  2 +-
>  security/landlock/ruleset.h |  2 +-
>  security/landlock/task.c    |  4 ++--
>  security/landlock/tsync.c   | 17 ++++++-----------
>  8 files changed, 25 insertions(+), 34 deletions(-)
> 
> diff --git a/security/landlock/cred.h b/security/landlock/cred.h
> index c10a06727eb1..f287c56b5fd4 100644
> --- a/security/landlock/cred.h
> +++ b/security/landlock/cred.h
> @@ -115,7 +115,7 @@ static inline bool landlocked(const struct task_struct *const task)
>   * @handle_layer: returned youngest layer handling a subset of @masks.  Not set
>   *                if the function returns NULL.
>   *
> - * Returns: landlock_cred(@cred) if any access rights specified in @masks is
> + * Return: landlock_cred(@cred) if any access rights specified in @masks is
>   * handled, or NULL otherwise.
>   */
>  static inline const struct landlock_cred_security *
> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> index 343a1aabaac6..8b9939005aa8 100644
> --- a/security/landlock/domain.c
> +++ b/security/landlock/domain.c
> @@ -34,7 +34,7 @@
>   * @exe_size: Returned size of @exe_str (including the trailing null
>   *            character), if any.
>   *
> - * Returns: A pointer to an allocated buffer where @exe_str point to, %NULL if
> + * Return: A pointer to an allocated buffer where @exe_str point to, %NULL if
>   * there is no executable path, or an error otherwise.
>   */
>  static const void *get_current_exe(const char **const exe_str,
> @@ -73,7 +73,7 @@ static const void *get_current_exe(const char **const exe_str,
>  }
>  
>  /*
> - * Returns: A newly allocated object describing a domain, or an error
> + * Return: A newly allocated object describing a domain, or an error
>   * otherwise.
>   */
>  static struct landlock_details *get_current_details(void)
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index cfe69075bf4e..a03ec664c78e 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -119,8 +119,8 @@ static const struct landlock_object_underops landlock_fs_underops = {
>   * Any new IOCTL commands that are implemented in fs/ioctl.c's do_vfs_ioctl()
>   * should be considered for inclusion here.
>   *
> - * Returns: true if the IOCTL @cmd can not be restricted with Landlock for
> - * device files.
> + * Return: True if the IOCTL @cmd can not be restricted with Landlock for
> + * device files, false otherwise.
>   */
>  static __attribute_const__ bool is_masked_device_ioctl(const unsigned int cmd)
>  {
> @@ -428,10 +428,10 @@ static bool may_refer(const struct layer_access_masks *const src_parent,
>   * Check that a destination file hierarchy has more restrictions than a source
>   * file hierarchy.  This is only used for link and rename actions.
>   *
> - * Returns: true if child1 may be moved from parent1 to parent2 without
> - * increasing its access rights.  If child2 is set, an additional condition is
> + * Return: True if child1 may be moved from parent1 to parent2 without
> + * increasing its access rights (if child2 is set, an additional condition is
>   * that child2 may be used from parent2 to parent1 without increasing its access
> - * rights.
> + * rights), false otherwise.
>   */
>  static bool no_more_access(const struct layer_access_masks *const parent1,
>  			   const struct layer_access_masks *const child1,
> @@ -734,9 +734,7 @@ static void test_is_eacces_with_write(struct kunit *const test)
>   * checks that the collected accesses and the remaining ones are enough to
>   * allow the request.
>   *
> - * Returns:
> - * - true if the access request is granted;
> - * - false otherwise.
> + * Return: True if the access request is granted, false otherwise.
>   */
>  static bool
>  is_access_to_paths_allowed(const struct landlock_ruleset *const domain,
> @@ -1022,9 +1020,8 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
>   * only handles walking on the same mount point and only checks one set of
>   * accesses.
>   *
> - * Returns:
> - * - true if all the domain access rights are allowed for @dir;
> - * - false if the walk reached @mnt_root.
> + * Return: True if all the domain access rights are allowed for @dir, false if
> + * the walk reached @mnt_root.
>   */
>  static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
>  				    const struct dentry *const mnt_root,
> @@ -1120,10 +1117,9 @@ static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
>   * ephemeral matrices take some space on the stack, which limits the number of
>   * layers to a deemed reasonable number: 16.
>   *
> - * Returns:
> - * - 0 if access is allowed;
> - * - -EXDEV if @old_dentry would inherit new access rights from @new_dir;
> - * - -EACCES if file removal or creation is denied.
> + * Return: 0 if access is allowed, -EXDEV if @old_dentry would inherit new
> + * access rights from @new_dir, or -EACCES if file removal or creation is
> + * denied.
>   */
>  static int current_check_refer_path(struct dentry *const old_dentry,
>  				    const struct path *const new_dir,
> diff --git a/security/landlock/id.c b/security/landlock/id.c
> index 838c3ed7bb82..6c8769777fdc 100644
> --- a/security/landlock/id.c
> +++ b/security/landlock/id.c
> @@ -258,7 +258,7 @@ static void test_range2_rand16(struct kunit *const test)
>   *
>   * @number_of_ids: Number of IDs to hold.  Must be greater than one.
>   *
> - * Returns: The first ID in the range.
> + * Return: The first ID in the range.
>   */
>  u64 landlock_get_id_range(size_t number_of_ids)
>  {
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index de8386af2f30..52e48ffcc3aa 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -675,7 +675,7 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
>   * @masks: Layer access masks to populate.
>   * @key_type: The key type to switch between access masks of different types.
>   *
> - * Returns: An access mask where each access right bit is set which is handled
> + * Return: An access mask where each access right bit is set which is handled
>   * in any of the active layers in @domain.
>   */
>  access_mask_t
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index 87d52031fb5a..5e63f78f7e1a 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -232,7 +232,7 @@ static inline void landlock_get_ruleset(struct landlock_ruleset *const ruleset)
>   *
>   * @domain: Landlock ruleset (used as a domain)
>   *
> - * Returns: an access_masks result of the OR of all the domain's access masks.
> + * Return: An access_masks result of the OR of all the domain's access masks.
>   */
>  static inline struct access_masks
>  landlock_union_access_masks(const struct landlock_ruleset *const domain)
> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index bf7c3db7ce46..f2dbdebf2770 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> @@ -174,8 +174,8 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
>   * @server: IPC receiver domain.
>   * @scope: The scope restriction criteria.
>   *
> - * Returns: True if @server is in a different domain from @client, and @client
> - * is scoped to access @server (i.e. access should be denied).
> + * Return: True if @server is in a different domain from @client and @client
> + * is scoped to access @server (i.e. access should be denied), false otherwise.
>   */
>  static bool domain_is_scoped(const struct landlock_ruleset *const client,
>  			     const struct landlock_ruleset *const server,
> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> index b06a0fa4cedb..359aecbb1e4b 100644
> --- a/security/landlock/tsync.c
> +++ b/security/landlock/tsync.c
> @@ -183,10 +183,8 @@ struct tsync_works {
>   * capacity.  This can legitimately happen if new threads get started after we
>   * grew the capacity.
>   *
> - * Returns:
> - *   A pointer to the preallocated context struct, with task filled in.
> - *
> - *   NULL, if we ran out of preallocated context structs.
> + * Return: A pointer to the preallocated context struct with task filled in, or
> + * NULL if preallocated context structs ran out.
>   */
>  static struct tsync_work *tsync_works_provide(struct tsync_works *s,
>  					      struct task_struct *task)
> @@ -243,11 +241,8 @@ static void tsync_works_trim(struct tsync_works *s)
>   * On a successful return, the subsequent n calls to tsync_works_provide() are
>   * guaranteed to succeed.  (size + n <= capacity)
>   *
> - * Returns:
> - *   -ENOMEM if the (re)allocation fails
> -
> - *   0       if the allocation succeeds, partially succeeds, or no reallocation
> - *           was needed
> + * Return: 0 on success or partial success, -ENOMEM if the (re)allocation
> + * fails.

tsync_works_grow_by:

I don't know what I meant when I wrote "partially succeeds" here in
the original patch.  Would suggest this phrasing:

  Return: 0 if sufficient space for n more elements could be provided,
  -ENOMEM on allocation errors, -EOVERFLOW in case of integer
  overflow.

With this function, the success criterium is that it can establish
that invariant.  We also don't return a success if we only could
allocate space for fewer elements.

>   */
>  static int tsync_works_grow_by(struct tsync_works *s, size_t n, gfp_t flags)
>  {
> @@ -363,8 +358,8 @@ static size_t count_additional_threads(const struct tsync_works *works)
>   * For each added task_work, atomically increments shared_ctx->num_preparing and
>   * shared_ctx->num_unfinished.
>   *
> - * Returns:
> - *     true, if at least one eligible sibling thread was found
> + * Return: True if at least one eligible sibling thread was found, false
> + * otherwise.
>   */
>  static bool schedule_task_work(struct tsync_works *works,
>  			       struct tsync_shared_context *shared_ctx)
> -- 
> 2.53.0
> 

Reviewed-by: Günther Noack <gnoack3000@gmail.com>