Re: [PATCH] drivers/net/wan/hdlc_ppp: fix potential null pointer in ppp_cp_event logging

[Paolo Abeni <pabeni@redhat.com>]

On 10/7/25 11:28 AM, Krzysztof Hałasa wrote:
> Paolo Abeni <pabeni@redhat.com> writes:
>> If v2 is not ready yet, I think it would be better returning "unknown"
>> instead of "LCP" when the protocol id is actually unknown.
>>
>> In the current code base, such case is unexpected/impossible, but the
>> compiler force us to handle it anyway. I think we should avoid hiding
>> the unexpected event.
>>
>> Assuming all the code paths calling proto_name() ensure the pid is a
>> valid one, you should possibly add a WARN_ONCE() on the default case.
> 
> Look, this is really simple code. Do we need additional bloat
> everywhere?
> 
> The compiler doesn't force us to anything. We define that, as far as
> get_proto() is concerned, PID_IPCP is "IPCP", PID_IPV6CP is "IPV6CP",
> and all other values mean "LCP". Then we construct the switch statement
> accordingly. Well, it seems I failed it slightly originally, most
> probably due to copy & paste from get_proto(). Now Kriish has noticed it
> and agreed to make it perfect.
> 
> Do you really think we should now change semantics of this 20 years old
> code (most probably never working incorrectly), adding some "unknown"
> (yet impossible) case, and WARNing about a condition which is excluded
> at the start of the whole RX parser?

Note that the suggested change is not going to change any semantic, just
make it clear for future changes that such case is not really expected.

And that in turn is my point. If someone else is going to touch this
code in the (not so near) future, such person will not have to read all
the possible code paths leading to proto_name() to understand the
assumption in the current code base.

Cheers,

Paolo