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

Ulisses Furquim
2012-03-20T12:52:22Z

Hi Andrei,

On Tue, Mar 20, 2012 at 9:49 AM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> 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".

If we'll have dozens of "if/else" then the separation is still not
good IMO. Right?

Regards,

--
Ulisses Furquim
ProFUSION embedded systems
http://profusion.mobi
Mobile: +55 19 9250 0942
Skype: ulissesffs