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
| ||
|
Date: Mon, 9 Nov 2020 16:38:21 +0100 From: Oliver Hartkopp <socketcan@...tkopp.net> To: Vincent MAILHOL <mailhol.vincent@...adoo.fr> Cc: linux-can <linux-can@...r.kernel.org>, Marc Kleine-Budde <mkl@...gutronix.de>, netdev <netdev@...r.kernel.org> Subject: Re: [PATCH v4 4/7] can: replace can_dlc as variable/element for payload length Hi Vincent, On 09.11.20 13:59, Vincent MAILHOL wrote: > On Mon. 9 Nov 2020 at 19:26, Oliver Hartkopp wrote: >> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h >> index b2e8df8e4cb0..72671184a7a2 100644 >> --- a/include/linux/can/dev.h >> +++ b/include/linux/can/dev.h >> @@ -183,12 +183,12 @@ static inline void can_set_static_ctrlmode(struct net_device *dev, >> /* override MTU which was set by default in can_setup()? */ >> if (static_mode & CAN_CTRLMODE_FD) >> dev->mtu = CANFD_MTU; >> } >> >> -/* get data length from can_dlc with sanitized can_dlc */ >> -u8 can_dlc2len(u8 can_dlc); >> +/* get data length from raw data length code (DLC) */ > > /* > * convert a given data length code (dlc) of an FD CAN frame into a > * valid data length of max. 64 bytes. > */ > > I missed this point during my previous review: the can_dlc2len() function > is only valid for CAN FD frames. Comments should reflect this fact. > >> +u8 can_dlc2len(u8 dlc); > > Concerning the name: > * can_get_cc_len() converts a Classical CAN frame DLC into a data > length. > * can_dlc2len() converts an FD CAN frame DLC into a data length. > > Just realized that both macro/function do similar things so we could > think of a similar naming as well. > * Example 1: can_get_cc_len() and can_get_fd_len() > * Example 2: can_cc_dlc2len() and can_fd_dlc2len() I like! Patch set v5 is out now. Thanks, Oliver > > Or we could simply leave things as they are, this is not a big issue > as long as the comments clearly state which one is for classical > frames and which one is for FD frames. > >> >> /* map the sanitized data length to an appropriate data length code */ >> u8 can_len2dlc(u8 len); > > can_len2dlc() might be renamed (e.g. can_get_fd_dlc()) if Example 1 > solution is chosen. > >> struct net_device *alloc_candev_mqs(int sizeof_priv, unsigned int echo_skb_max, > > Yours sincerely, > Vincent Mailhol >
Powered by blists - more mailing lists