On 4/5/19 3:07 PM, Kurt Van Dijck wrote:
> On vr, 05 apr 2019 14:07:43 +0200, Marc Kleine-Budde wrote:
>> On 4/5/19 11:46 AM, Kurt Van Dijck wrote:
>>> On vr, 05 apr 2019 08:33:52 +0200, David Jander wrote:
>>>> On Thu, 4 Apr 2019 19:54:30 +0200
>>>> Kurt Van Dijck <dev.kurt@xxxxxxxxxxxxxxxxxxxxxx> wrote:
>>>>
>>>>> On vr, 29 mrt 2019 14:58:38 +0100, Oleksij Rempel wrote:
>>>>>> diff --git a/net/can/j1939/j1939-priv.h b/net/can/j1939/j1939-priv.h
>>>>>> index df058d08fe68..127b4e28b16a 100644
>>>>>> --- a/net/can/j1939/j1939-priv.h
>>>>>> +++ b/net/can/j1939/j1939-priv.h
>>>>>> @@ -113,7 +113,7 @@ struct j1939_ecu *j1939_ecu_get_by_name_locked(struct j1939_priv *priv,
>>>>>> struct j1939_addr {
>>>>>> name_t src_name;
>>>>>> name_t dst_name;
>>>>>> - pgn_t pgn;
>>>>>> + pgn_t dst_pgn;
>>>>>>
>>>>>> u8 sa;
>>>>>> u8 da;
>>>>>
>>>>> I considered j1939_addr like a kind of label of a packet. In j1939, a
>>>>> packet has only 1 pgn.
>>>>> I'm curious why you need 2.
>>>>
>>>> There is only one. It just was renamed from pgn to dst_pgn, probably to make
>>>> clear that it is related to the destination and not the source (which wouldn't
>>>> make any sense).
>>>
>>> Well, the PGN describes the packet that the source sends to the destination,
>>> so it's equally related to the source as to the destination.
>>> renaming it didn't clarify for me, in contrary.
>>>
>>> I suppose it's nitpicking, so if you all think it's better
>>> understandable like that, then I'm ok too.
>>> It's not visible in userspace anyway.
>>
>> ...but the kernel code should be readable, too.
> that is true also :-)
>>
>> As said in the other mail the dst_pgn is the pgn label on the packets.
>> The "other" pgn, which is named pgn_rx_filter and member in the struct
>> j1939_sock, is set by bind() and only used to filter incoming packets.
>
> Oleksij created the right variables and the right code.
> Only the name 'dst_pgn' confuses me.
Naming things is the most difficult part of coding...right after proper
testing.
> The Engine RPM PGN, for example, is name named to its contents, and
> source, not destination. It's PGN number should not be called 'dst_pgn'.
> Any broadcasted PGN qualifies for this example, and non-broadcasted
> PGN's do not inherently justify the opposite.
>
>>> so it's equally related to the source as to the destination.
>
> It should neither be called 'src_pgn', just plain 'pgn'.
It's "pgn" then.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
Attachment:
signature.asc
Description: OpenPGP digital signature
