On Fri, Jan 15, 2021 at 01:38:12PM +0100, Oliver Hartkopp wrote:

The problem is NOT that we make sure to access this array correctly.

This particular array is no set of arbitrary values that may be extended
later on BUT it is a 'translation map' for defined length values which will
never change.

Therefore ARRAY_SIZE(array) hides the fact that every length value "greater
than 48" results to a DLC of 15.

For that reason my former code was very clear:

1. It had a table that mapped 0 .. 64 to a DLC
2. It had a test for '> 64' as sanity test.

I stole the idea of removing the last 16 bytes of the array from the padlen()
function in the ISOTP code.

Now the sanity test is gone and mixed up with the mapping of length values -
and finally with ARRAY_SIZE(whatever) which doesn't give a hint why this is
checked.

...then I wrongly replaced the 48 with the ARRAY_SIZE().

We are writing code to be understandable for the reader and the suggested
'improvement' which saves 16 bytes does exactly the opposite.

For that reason the entire patch is broken.

An improvement could be to rename

64 -> CANFD_MAX_LEN
0xF -> CANFD_MAX_DLC

but nothing more.

What about this patch:

diff --git a/drivers/net/can/dev/length.c b/drivers/net/can/dev/length.c
index d35c4e82314d..f5509e48fe95 100644
--- a/drivers/net/can/dev/length.c
+++ b/drivers/net/can/dev/length.c

@@ -27,12 +27,14 @@ static const u8 len2dlc[] = {
        13, 13, 13, 13, 13, 13, 13, 13, /* 25 - 32 */
        14, 14, 14, 14, 14, 14, 14, 14, /* 33 - 40 */
        14, 14, 14, 14, 14, 14, 14, 14, /* 41 - 48 */
+       15, 15, 15, 15, 15, 15, 15, 15, /* 49 - 56 */
+       15, 15, 15, 15, 15, 15, 15, 15  /* 57 - 64 */
 };

 /* map the sanitized data length to an appropriate data length code */
 u8 can_fd_len2dlc(u8 len)
 {
-       if (len >= ARRAY_SIZE(len2dlc))
+       if (unlikely(len > CANFD_MAX_LEN))
                return CANFD_MAX_DLC;

        return len2dlc[len];

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |