Messages in this thread | | | Date | Sun, 14 Apr 2024 22:21:33 +0200 | Subject | Re: [PATCH v2 1/1] Documentation: networking: document ISO 15765-2:2016 | From | Oliver Hartkopp <> |
| |
On 14.04.24 06:03, Vincent Mailhol wrote:
> > This doesn't remove the fact that I think that this naming convention > is stupid because of the RAS syndrome, but I acknowledge that CAN CC > is now the official denomination and thus, that we should adopt it in > our documentation as well. >
;-)
>>> Add a space between ISO and the number. Also, update the year: >>> >>> ISO 15765-2:2024 >>> >> >> Interesting! Didn't know there's already a new version. >> >> Will check this out whether we really support ISO 15765-2:2024 ... >> >> Do you have the standard at hand right now or should we leave this as >> ISO15765-2:2016 until we know? > > I have access to the newer revisions. But I never really invested time > into reading that standard (neither the 2016 nor the 2024 versions). > > Regardless, here is a verbatim extract from the Foreworld section of > ISO 15765-2:2024 > > This fourth edition cancels and replaces the third edition (ISO > 15765-2:2016), which has been technically revised. > > The main changes are as follows: > > - restructured the document to achieve compatibility with OSI > 7-layers model; > > - introduced T_Data abstract service primitive interface to > achieve compatibility with ISO 14229-2; > > - moved all transport layer protocol-related information to Clause 9; > > - clarification and editorial corrections >
Yes, I've checked the release notes on the ISO website too. This really looks like editorial stuff that has nothing to do with the data protocol and its segmentation.
>>> >>> Here, I would suggest the C99 designated field initialization: >>> >>> struct sockaddr_can addr = { >>> .can_family = AF_CAN; >>> .can_ifindex = if_nametoindex("can0"); >>> .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG; >>> .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG; >>> }; > > Typo in my previous message: the designated initializers are not > separated by colon ";" but by comma ",". So it should have been: > > struct sockaddr_can addr = { > .can_family = AF_CAN, > .can_ifindex = if_nametoindex("can0"), > .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG, > .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG, > }; > >>> Well, this is just a suggestion, feel free to reject it if you do not like it. >> >> At least I don't like it. >> >> These values are usually interactively given on the command line: >> >> > .can_ifindex = if_nametoindex("can0"); >> > .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG; >> > .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG; >> >> So have it in a static field initialization leads to a wrong path IMO. > > There is no such limitation that C99 designated initializers should > only work with variables which have static storage duration. In my > suggested example, nothing is static. > > I see this as the same thing as below example: > > int foo(void); > > int bar() > { > int i = foo(); > } > > int baz() > { > int i; > > i = foo(); > } > > In bar(), the fact that the variable i is initialized at declaration > does not mean that it is static. In both examples, the variable i uses > automatic storage duration. > > Here, my preference goes to bar(), but I recognize that baz() is also > perfectly fine. Replace the int type by the struct sockaddr_can type > and the scalar initialization by designated initializers and you > should see the connection.
Oh, sorry. Maybe I expressed myself wrong.
IMHO your way to work with an initializer is correct from the C standpoint.
But I think this is pretty unusual for a code example when an application programmer starts to work with ISO-TP.
You usually get most of these values from the command line an fill the struct _by hand_ - and not with a static initialization.
That was my suggestion.
> > ** Different topic ** > > While replying on this, I encountered something which made me worry a bit: > > The type of sockaddr_can.can_ifindex is a signed int: > > https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/can.h#L243 > > But if_nametoindex() returns an unsigned int: > > https://man7.org/linux/man-pages/man3/if_nametoindex.3.html > > Shouldn't sockaddr_can.can_ifindex also be declared as an unsigned int? >
The if_index derives from struct netdevice.if_index
https://elixir.bootlin.com/linux/v6.8.6/source/include/linux/netdevice.h#L2158
which is an int.
I don't think this would have an effect in real world to change sockaddr_can.can_ifindex to an unsigned int.
I wonder if it is more critical to existing user space code to change it to unsigned int or to leave it as-is ...
Best regards, Oliver
| |