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