Hi Octavian,
> >> >> +void bt_sock_reclassify_lock(struct sock *sk, int proto)
> >> >> {
> >> >> - struct sock *sk = sock->sk;
> >> >> -
> >> >> if (!sk)
> >> >> return;
> >> >
> >> > Why are we keeping the !sk check here if we already hand in the sk. It
> >> > is most likely checked by the caller already.
> >> >
> >>
> >> In rfcomm_sock_create (called from bt_sock_create) sock->sk can be set
> >> to NULL so I think we should keep the check.
> >
> > it has been too long since I looked at this part of the code. You need
> > to walk me through it why this is still true.
> >
>
> Hi Marcel,
>
> In bt_sock_create we have:
>
> if (bt_proto[proto] && try_module_get(bt_proto[proto]->owner)) {
> err = bt_proto[proto]->create(net, sock, proto, kern);
> bt_sock_reclassify_lock(sock->sk, proto);
>
> and create can be rfcomm_sock_create where we have
>
> sk = rfcomm_sock_alloc(net, sock, protocol, GFP_ATOMIC);
> if (!sk)
> return -ENOMEM;
>
> So after calling ->create() sock->sk can be NULL and thus we can call
> bt_sock_reclassify with a NULL parameter.
so instead of keep checking sock->sk multiple times in different places,
why not actually check the error from bt_proto[]->create() and skip the
reclassify_lock call.
And if we wanna be really paranoid, then lets add a BUG_ON for sk so we
get some nice backtraces if we have a wrongful caller.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
