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]
Date: Fri, 19 Jan 2024 09:34:36 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Jacob Lott <jklott.git@...il.com>
Cc: gregkh@...uxfoundation.org, linux-staging@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] staging: rts5208: Add more details to Kconfig help

On Thu, Jan 18, 2024 at 02:17:04PM -0500, Jacob Lott wrote:
> The current help text is short and triggers a
> warning from checkpatch.pl. This patch adds more
> details to the help text which should provide better
> information for whether or not to enable the driver.
> 
> Signed-off-by: Jacob Lott <jklott.git@...il.com>
> ---
> v2 -> v3: Fixed spacing and newline in text

The commit message is still not line wrapped correctly at 72-75
characters.  It should have been:

The current help text is short and triggers a warning from checkpatch.pl.
This patch adds more details to the help text which should provide better
information for whether or not to enable the driver.

Or:

The current help text is short and triggers a warning from
checkpatch.pl. This patch adds more details to the help text which
should provide better information for whether or not to enable the
driver.


> v1 -> v2: Corrected line spacing based off feedback
> 
>  drivers/staging/rts5208/Kconfig | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/rts5208/Kconfig b/drivers/staging/rts5208/Kconfig
> index b864023d3ccb..c4664a26ba3b 100644
> --- a/drivers/staging/rts5208/Kconfig
> +++ b/drivers/staging/rts5208/Kconfig
> @@ -3,7 +3,11 @@ config RTS5208
>  	tristate "Realtek PCI-E Card Reader RTS5208/5288 support"
>  	depends on PCI && SCSI
>  	help
> -	  Say Y here to include driver code to support the Realtek
> -	  PCI-E card reader rts5208/rts5288.
> +	  Choose Y here to enable support for the Realtek PCI-E card reader
> +	  RTS5208/5288. This driver facilitates communication between the Linux
> +	  kernel and the Realtek PCI-E card reader.
>  
> -	  If this driver is compiled as a module, it will be named rts5208.
> +	  If you opt to compile this driver as a module, it will be named rts5208.
> +	  Selecting N will exclude this driver from the kernel build. Choose option
> +	  Y if your system includes the Realtek PCI-E card reader rts5208/rts5288.
> +	  When in doubt, it is generally safe to select N.


In the original code there were three lines of help text.

1)         Say Y here to include driver code to support the Realtek
2)         PCI-E card reader rts5208/rts5288.
3)         If this driver is compiled as a module, it will be named rts5208.

The minimum that checkpatch wants is 4 lines.

However, I feel like nothing you're adding here is useful information.
Some of it is just confusing like, "This driver facilitates
communication between the Linux kernel and the Realtek PCI-E card
reader."  That line feels like information, but when you think about it,
it doesn't actually mean anything.  The second paragraph is just
repeated information so it's kind of a waste of time.

Don't just obey checkpatch for the sake of obeying, if we can't think
of anything else useful to say then three lines is enough.

regards,
dan carpenter


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ