linux-kernel - Re: [PATCH v2] power: swap: Convert kernel-doc comments to regular block comments

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] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0jEqNPPNE-5FuAm3Hd8YH1BecoOACoa6Sdr+VjHwBk9PA@mail.gmail.com>
Date: Fri, 14 Nov 2025 17:37:17 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Sunday Adelodun <adelodunolaoluwa@...oo.com>
Cc: rafael@...nel.org, pavel@...nel.org, lenb@...nel.org, 
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	skhan@...uxfoundation.org, david.hunter.linux@...il.com, 
	linux-kernel-mentees@...ts.linuxfoundation.org
Subject: Re: [PATCH v2] power: swap: Convert kernel-doc comments to regular
 block comments

On Thu, Nov 13, 2025 at 12:31 PM Sunday Adelodun
<adelodunolaoluwa@...oo.com> wrote:
>
> Several static functions in kernel/power/swap.c were using the kernel-doc
> comment style (/** ... */) even though they are not exported or referenced
> in generated documentation. This triggers documentation warnings and makes
> the comments inconsistent with kernel style for local functions.
>
> Convert these comment blocks to regular C-style comments (/* ... */) and
> update a few function headers to include proper "Return:" descriptions
> where applicable.
>
> No functional changes.
>
> Signed-off-by: Sunday Adelodun <adelodunolaoluwa@...oo.com>
> ---
> changelog:
>
> changes from v1 to v2:
> - Converted function comment blocks from /** */ to /* */ style for
>   static functions
> - Minor reformatting of comment indentation and spacing
>
> v1 patch link:
> https://lore.kernel.org/all/20251106113938.34693-2-adelodunolaoluwa@yahoo.com/
>
>  kernel/power/swap.c | 54 ++++++++++++++++++++-------------------------
>  1 file changed, 24 insertions(+), 30 deletions(-)
>
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index 0beff7eeaaba..076ed590e8c9 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -336,10 +336,8 @@ static int mark_swapfiles(struct swap_map_handle *handle, unsigned int flags)
>   */
>  unsigned int swsusp_header_flags;
>
> -/**
> - *     swsusp_swap_check - check if the resume device is a swap device
> - *     and get its index (if so)
> - *
> +/*
> + *     check if the resume device is a swap device and get its index (if so).
>   *     This is called before saving image
>   */

I'd move this comment (after the change above) into the function body
(before the first statement) because the function does more than what
is says.

>  static int swsusp_swap_check(void)
> @@ -362,11 +360,8 @@ static int swsusp_swap_check(void)
>         return 0;
>  }
>
> -/**
> - *     write_page - Write one page to given swap location.
> - *     @buf:           Address we're writing.
> - *     @offset:        Offset of the swap page we're writing to.
> - *     @hb:            bio completion batch
> +/*
> + *     Write one page to given swap location.
>   */

There is not much value in the comment after the change, please drop
it altogether.

>  static int write_page(void *buf, sector_t offset, struct hib_bio_batch *hb)
> @@ -526,8 +521,8 @@ static int swap_writer_finish(struct swap_map_handle *handle,
>  #define CMP_MIN_RD_PAGES       1024
>  #define CMP_MAX_RD_PAGES       8192
>
> -/**
> - *     save_image - save the suspend image data
> +/*
> + *     save the suspend image data
>   */

Same here.

>  static int save_image(struct swap_map_handle *handle,
> @@ -671,11 +666,8 @@ static int compress_threadfn(void *data)
>         return 0;
>  }
>
> -/**
> - * save_compressed_image - Save the suspend image data after compression.
> - * @handle: Swap map handle to use for saving the image.
> - * @snapshot: Image to read data from.
> - * @nr_to_write: Number of pages to save.
> +/*
> + * Save the suspend image data after compression.
>   */

Same here.

>  static int save_compressed_image(struct swap_map_handle *handle,
>                                  struct snapshot_handle *snapshot,
> @@ -904,11 +896,8 @@ static int save_compressed_image(struct swap_map_handle *handle,
>         return ret;
>  }
>
> -/**
> - *     enough_swap - Make sure we have enough swap to save the image.
> - *
> - *     Returns TRUE or FALSE after checking the total amount of swap
> - *     space available from the resume partition.
> +/*
> + *     Make sure we have enough swap to save the image.
>   */

Same here.

>  static int enough_swap(unsigned int nr_pages)
> @@ -930,6 +919,8 @@ static int enough_swap(unsigned int nr_pages)
>   *     them synced (in case something goes wrong) but we DO not want to mark
>   *     filesystem clean: it is not. (And it does not matter, if we resume
>   *     correctly, we'll mark system clean, anyway.)
> + *
> + *     Return: 0 on success, negative error code on failure.
>   */
>

Please remove the empty line between the comment and the function definition.

>  int swsusp_write(unsigned int flags)
> @@ -1077,10 +1068,8 @@ static int swap_reader_finish(struct swap_map_handle *handle)
>         return 0;
>  }
>
> -/**
> - *     load_image - load the image using the swap map handle
> - *     @handle and the snapshot handle @snapshot
> - *     (assume there are @nr_pages pages to load)
> +/*
> + *     load the image using the swap map handle
>   */
>
>  static int load_image(struct swap_map_handle *handle,
> @@ -1190,11 +1179,8 @@ static int decompress_threadfn(void *data)
>         return 0;
>  }
>
> -/**
> - * load_compressed_image - Load compressed image data and decompress it.
> - * @handle: Swap map handle to use for loading data.
> - * @snapshot: Image to copy uncompressed data into.
> - * @nr_to_read: Number of pages to load.
> +/*
> + * Load compressed image data and decompress it.
>   */

This comment is not too useful any more, please drop it.

>  static int load_compressed_image(struct swap_map_handle *handle,
>                                  struct snapshot_handle *snapshot,
> @@ -1529,6 +1515,8 @@ static int load_compressed_image(struct swap_map_handle *handle,
>   *     swsusp_read - read the hibernation image.
>   *     @flags_p: flags passed by the "frozen" kernel in the image header should
>   *               be written into this memory location
> + *
> + *     Return: 0 on success, negative error code on failure.
>   */
>

Please remove the empty line between the comment and the function definition.

>  int swsusp_read(unsigned int *flags_p)
> @@ -1567,6 +1555,10 @@ static void *swsusp_holder;
>  /**
>   * swsusp_check - Open the resume device and check for the swsusp signature.
>   * @exclusive: Open the resume device exclusively.
> + *
> + * Return:
> + *     0 if a valid hibernation image is found,
> + *     negative error code on failure.

I think that the above can be one line, can't it?

>   */

Please remove the empty line between the comment and the function definition.

>  int swsusp_check(bool exclusive)
> @@ -1631,6 +1623,8 @@ void swsusp_close(void)
>
>  /**
>   *      swsusp_unmark - Unmark swsusp signature in the resume device
> + *
> + *      Return: 0 on success, negative error code on failure.
>   */

Same here.

>  #ifdef CONFIG_SUSPEND
> --

Thanks!