Re: [RFCv4 05/30] Bluetooth: Lock sk only if exist

Andrei Emeltchenko
2012-03-20T12:49:26Z

Hi Ulisses,

On Tue, Mar 20, 2012 at 09:22:46AM -0300, Ulisses Furquim wrote:
> > @@ -212,9 +212,13 @@ static inline void l2cap_state_change(struct l2cap_chan *chan, int state, int er
> >  {
> >        struct sock *sk = chan->sk;
> >
> > -       lock_sock(sk);
> > +       if (sk)
> > +               lock_sock(sk);
> > +
> >        __l2cap_state_change(chan, state, err);
> > -       release_sock(sk);
> > +
> > +       if (sk)
> > +               release_sock(sk);
> >  }
> >
> >  void __l2cap_chan_set_err(struct l2cap_chan *chan, int err)
> > --
> > 1.7.9.1
>
> Well, this doesn't look good, does it? Wouldn't make sense to call
> __l2cap_state_change() where we know sk doesn't exist and
> l2cap_state_change() in the others? After all the separation between
> chan and sk is something we need to have as much clear as possible
> from now on, right?

Sounds good, the only issue is that instead of this simple change we would
have dozens of "if/else".

Best regards
Andrei Emeltchenko