linux-kernel - Re: [PATCH v5 1/9] w1_therm: adding code comments and code reordering

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: <eb73984b-8317-a0e9-90ed-2956d3fbc6b0@infradead.org>
Date:   Sat, 9 May 2020 19:26:25 -0700
From:   Randy Dunlap <rdunlap@...radead.org>
To:     Akira Shimahara <akira215corp@...il.com>, greg@...ah.com
Cc:     zbr@...emap.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/9] w1_therm: adding code comments and code reordering

Hi--

On 5/9/20 4:57 PM, Akira Shimahara wrote:
> Adding code comments to split code in dedicated parts. After the global
> declarations (defines, macros and function declarations), code is organized
> as follow :
>  - Device and family dependent structures and functions
>  - Interfaces functions
>  - Helpers functions
>  - Hardware functions
>  - Sysfs interface functions
> 
> Signed-off-by: Akira Shimahara <akira215corp@...il.com>
> ---
> Main motivation on the first patch of this serie is to clean up the code,
> document it and reorder it to prepare the next patches, which are clearer
> after this.
> 
> One main point is to keep all device/family dependent code gather at the
> beginning to ease adding new devices if required.
> 
> Changes in v5:
> - All patch serie in one .c file
> - Correcting some comments
> - adding <linux/string.h> include
> 
>  drivers/w1/slaves/w1_therm.c | 403 ++++++++++++++++++++---------------
>  1 file changed, 237 insertions(+), 166 deletions(-)
> 
> diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
> index 18f08d7..f7147b2 100644
> --- a/drivers/w1/slaves/w1_therm.c
> +++ b/drivers/w1/slaves/w1_therm.c
> @@ -41,55 +41,99 @@
>  static int w1_strong_pullup = 1;
>  module_param_named(strong_pullup, w1_strong_pullup, int, 0);
>  
> +/*-------------------------------Macros-------------------------------------*/
> +
> +/* return the address of the refcnt in the family data */
> +#define THERM_REFCNT(family_data) \
> +	(&((struct w1_therm_family_data *)family_data)->refcnt)
> +
> +/*----------------------------------Structs---------------------------------*/
> +
> +/**
> + * struct w1_therm_family_converter
> + * \brief Used to bind standard function call
> + * to device specific function
> + * it could be routed to NULL if device don't support feature
> + */

Hi,

All of the struct and function documentation comments in all patches in
this patch series should be using kernel-doc notation instead of the above
(whatever it is; I don't know what it is).


> +struct w1_therm_family_converter {
> +	u8		broken;
> +	u16		reserved;
> +	struct w1_family	*f;
> +	int		(*convert)(u8 rom[9]);
> +	int		(*precision)(struct device *device, int val);
> +	int		(*eeprom)(struct device *device);
> +};
> +

thanks.
-- 
~Randy