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.
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'.
Kind regards,
Kurt
