Re: [PATCH mptcp-next v1] mptcp: fix cast type of is_fully_established

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 26/03/2025 02:08, Geliang Tang wrote:
> Hi Matt,
> 
> Thanks for the review.
> 
> On Tue, 2025-03-25 at 11:49 +0100, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 25/03/2025 11:36, Geliang Tang wrote:
>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>
>>> The parameter type of mptcp_is_fully_established() is 'struct sock
>>> *',
>>> not 'void *', so this patch casts 'msk' as 'struct sock *' instead
>>> of
>>> 'void *' in mptcp_can_accept_new_subflow().
>>
>> Indeed, but it is not really wrong to use 'void *'. Also, it looks
>> like
>> we are doing that in most other files from the net/mptcp directory.
> 
> I just checked, and there're indeed 6 other places that use '(void
> *)msk' too, but more places (98) use '(struct sock *)msk'.
> 
> $ grep -r "(void \*)msk" net/mptcp/ | wc -l
> 7
> $ grep -r "(struct sock \*)msk" net/mptcp/ | wc -l
> 98
> 
> I'm wondering if we should change all 7 '(void *)msk' to '(struct sock
> *)msk' in v2 to be consistent.

Maybe, I don't know to be honest. There are other (void *) usages, and
they are "fine" for the moment. Changing them might be good for the
consistency, but annoying in case of backports :-/

>> If you have to modify this line for another commit, feel free to
>> include
>> the modification in this commit if it bothers you ("While at it, use
>> the
>> proper cast instead of the generic 'void *'"), but it can also be
>> left
>> unchanged I think.
> 
> Or simply drop this patch. Both are OK to me.

Yes, maybe. I will ask Mat what he thinks about that.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.