linux-kernel - Re: [PATCH v6 4/7] can: Add Nuvoton NCT6694 CANFD support

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] [day] [month] [year] [list]

Message-ID: <c255fe16-da67-42ff-86f3-89c1ac683c60@wanadoo.fr>
Date: Thu, 6 Feb 2025 20:05:26 +0900
From: Vincent Mailhol <mailhol.vincent@...adoo.fr>
To: Ming Yu <a0282524688@...il.com>
Cc: linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
 linux-i2c@...r.kernel.org, linux-can@...r.kernel.org,
 netdev@...r.kernel.org, linux-watchdog@...r.kernel.org,
 linux-hwmon@...r.kernel.org, linux-rtc@...r.kernel.org,
 linux-usb@...r.kernel.org, tmyu0@...oton.com, lee@...nel.org,
 linus.walleij@...aro.org, brgl@...ev.pl, andi.shyti@...nel.org,
 mkl@...gutronix.de, andrew+netdev@...n.ch, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
 wim@...ux-watchdog.org, linux@...ck-us.net, jdelvare@...e.com,
 alexandre.belloni@...tlin.com
Subject: Re: [PATCH v6 4/7] can: Add Nuvoton NCT6694 CANFD support

On 04/02/2025 at 12:24, Ming Yu wrote:
> Dear Vincent,
> 
> Thank you for reviewing,
> I will address the issues you mentioned in the next patch.
> 
> Vincent Mailhol <mailhol.vincent@...adoo.fr> 於 2025年1月26日 週日 下午4:47寫道：
>>
> ...
>>> +static int nct6694_can_get_clock(struct nct6694_can_priv *priv)
>>> +{
>>> +     struct nct6694_can_information *info;
>>> +     struct nct6694_cmd_header cmd_hd = {
>>
>> If the variable only has constant initializer, make it static const:
>>
>>         static const struct nct6694_cmd_header cmd_hd = {
>>
>> Apply this at other locations in your different modules.
>>
>>> +             .mod = NCT6694_CAN_MOD,
>>> +             .cmd = NCT6694_CAN_INFORMATION,
>>> +             .sel = NCT6694_CAN_INFORMATION_SEL,
>>> +             .len = cpu_to_le16(sizeof(*info))
>>> +     };
>>> +     int ret, can_clk;
>>> +
>>> +     info = kzalloc(sizeof(*info), GFP_KERNEL);
>>> +     if (!info)
>>> +             return -ENOMEM;
>>> +
> 
> Excuse me, I would like to confirm, if the variable is constant
> initializer, should the declaration be written as:
> static const struct nct6694_cmd_header cmd_hd = {
>     .mod = NCT6694_CAN_MOD,
>     .cmd = NCT6694_CAN_INFORMATION,
>     .sel = NCT6694_CAN_INFORMATION_SEL,
>     .len = cpu_to_le16(sizeof(struct nct6694_can_information))
> };
> instead of:
> static const struct nct6694_cmd_header cmd_hd = {
>     .mod = NCT6694_CAN_MOD,
>     .cmd = NCT6694_CAN_INFORMATION,
>     .sel = NCT6694_CAN_INFORMATION_SEL,
>     .len = cpu_to_le16(sizeof(*info))
> };
> , correct?

The sizeof() keyword returns a integer constant expression even if
applied on to a variable (unless that variable is a variable length
array, but these are banned in the kernel anyway). This is because only
the type of the variable matters, and that is known at compile time.

So, cpu_to_le16(sizeof(*info)) should work fine. Or are you getting any
error?

> In addition, does this mean that the parameter in nct6694_read_msg()
> and nct6694_write_msg() should be changed to const struct
> nct6694_cmd_header *cmd_hd?

Yes!


Yours sincerely,
Vincent Mailhol