All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vincent Mailhol <mailhol@kernel.org>
To: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: "Oliver Hartkopp" <socketcan@hartkopp.net>,
	"Stéphane Grosjean" <stephane.grosjean@hms-networks.com>,
	"Robert Nawrath" <mbro1689@gmail.com>,
	"Minh Le" <minh.le.aj@renesas.com>,
	"Duy Nguyen" <duy.nguyen.rh@renesas.com>,
	linux-can@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 04/20] can: netlink: refactor can_validate_bittiming()
Date: Wed, 10 Sep 2025 20:12:27 +0900	[thread overview]
Message-ID: <f6fbc5cc-392d-4914-a08c-e70091ca3371@kernel.org> (raw)
In-Reply-To: <20250910-quaint-weightless-narwhal-658e26-mkl@pengutronix.de>

On 10/09/2025 at 19:55, Marc Kleine-Budde wrote:
> On 10.09.2025 15:43:00, Vincent Mailhol wrote:
>> On 10/09/2025 at 15:13, Marc Kleine-Budde wrote:
>>> On 10.09.2025 15:03:29, Vincent Mailhol wrote:
>>>> Whenever can_validate_bittiming() is called, it is always preceded by
>>>> some boilerplate code which was copy pasted all over the place. Move
>>>> that repeated code directly inside can_validate_bittiming().
>>>>
>>>> Finally, the mempcy() is not needed. Just use the pointer returned by
>>>> nla_data() as-is.
>>>
>>> The memcpy()'ed struct is guaranteed to be properly aligned, is this
>>> also the case for the casted nla_data() pointer?
>>
>> The NLA attributes are aligned on 4 bytes, c.f. NLA_ALIGNTO:
>>
>> https://elixir.bootlin.com/linux/v6.16.5/source/include/uapi/linux/netlink.h#L248
>>
>> Which is sufficient for struct can_bittiming which also requires just 4 bytes of
>> alignment as proven by the fact the the code would still compile if I add this
>> static assert:
>>
>>   static_assert(_Alignof(typeof(*bt)) <= NLA_ALIGNTO);
>>
>> But I have to admit that you caught me off guard. I did not think of that. Maybe
>> I should add above static assertions to the code to document that what we are
>> doing is correct?
> 
> Yes, make it so!

I applied the changes locally.

Let me know when you are done with the review of the other patches. I will wait
for your other comments (if any) before sending v3.


Yours sincerely,
Vincent Mailhol


  reply	other threads:[~2025-09-10 11:12 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-10  6:03 [PATCH v2 00/20] can: netlink: preparation before introduction of CAN XL step 2/2 Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 01/20] can: dev: move struct data_bittiming_params to linux/can/bittiming.h Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 02/20] can: dev: make can_get_relative_tdco() FD agnostic and move it to bittiming.h Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 03/20] can: netlink: document which symbols are FD specific Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 04/20] can: netlink: refactor can_validate_bittiming() Vincent Mailhol
2025-09-10  6:13   ` Marc Kleine-Budde
2025-09-10  6:43     ` Vincent Mailhol
2025-09-10 10:55       ` Marc Kleine-Budde
2025-09-10 11:12         ` Vincent Mailhol [this message]
2025-09-10  6:03 ` [PATCH v2 05/20] can: netlink: add can_validate_tdc() Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 06/20] can: netlink: add can_validate_databittiming() Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 07/20] can: netlink: refactor CAN_CTRLMODE_TDC_{AUTO,MANUAL} flag reset logic Vincent Mailhol
2025-09-20  7:24   ` Vincent Mailhol
2025-09-22  9:43     ` Marc Kleine-Budde
2025-09-22 11:14       ` Vincent Mailhol
2025-09-22 13:06         ` Marc Kleine-Budde
2025-09-23  7:04           ` Vincent Mailhol
2025-09-23  9:08             ` Marc Kleine-Budde
2025-09-23  9:36               ` Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 08/20] can: netlink: remove useless check in can_tdc_changelink() Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 09/20] can: netlink: make can_tdc_changelink() FD agnostic Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 10/20] can: netlink: add can_dtb_changelink() Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 11/20] can: netlink: add can_ctrlmode_changelink() Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 12/20] can: netlink: make can_tdc_get_size() FD agnostic Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 13/20] can: netlink: add can_data_bittiming_get_size() Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 14/20] can: netlink: add can_bittiming_fill_info() Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 15/20] can: netlink: add can_bittiming_const_fill_info() Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 16/20] can: netlink: add can_bitrate_const_fill_info() Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 17/20] can: netlink: make can_tdc_fill_info() FD agnostic Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 18/20] can: calc_bittiming: make can_calc_tdco() " Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 19/20] can: dev: add can_get_ctrlmode_str() Vincent Mailhol
2025-09-10  6:03 ` [PATCH v2 20/20] can: netlink: add userland error messages Vincent Mailhol

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f6fbc5cc-392d-4914-a08c-e70091ca3371@kernel.org \
    --to=mailhol@kernel.org \
    --cc=duy.nguyen.rh@renesas.com \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mbro1689@gmail.com \
    --cc=minh.le.aj@renesas.com \
    --cc=mkl@pengutronix.de \
    --cc=socketcan@hartkopp.net \
    --cc=stephane.grosjean@hms-networks.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.