public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
From: Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com>
To: Ulisses Furquim <ulisses@profusion.mobi>
Cc: linux-bluetooth@vger.kernel.org
Subject: Re: [RFCv4 05/30] Bluetooth: Lock sk only if exist
Date: Tue, 20 Mar 2012 14:49:26 +0200	[thread overview]
Message-ID: <20120320124925.GE27063@aemeltch-MOBL1> (raw)
In-Reply-To: <CAA37ikb7ayXRER=JRZqA+gyj56-L1SA1g8FoOsdSEuLHcykcbg@mail.gmail.com>

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 

  reply	other threads:[~2012-03-20 12:49 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-15 12:29 [RFCv4 00/30] RFC Bluetooth A2MP implementation Andrei Emeltchenko
2012-03-15 12:29 ` [RFCv4 01/30] Bluetooth: Make ertm_init available Andrei Emeltchenko
2012-03-20  3:04   ` Gustavo Padovan
2012-03-20  8:03     ` Andrei Emeltchenko
2012-03-15 12:29 ` [RFCv4 02/30] Bluetooth: Add send function to chan ops Andrei Emeltchenko
2012-03-15 12:29 ` [RFCv4 03/30] Bluetooth: Make l2cap_chan_add available Andrei Emeltchenko
2012-03-15 12:29 ` [RFCv4 04/30] Bluetooth: Add set_err to state_change callback Andrei Emeltchenko
2012-03-15 12:29 ` [RFCv4 05/30] Bluetooth: Lock sk only if exist Andrei Emeltchenko
2012-03-20  3:03   ` Gustavo Padovan
2012-03-20 12:22   ` Ulisses Furquim
2012-03-20 12:49     ` Andrei Emeltchenko [this message]
2012-03-20 12:52       ` Ulisses Furquim
2012-03-15 12:29 ` [RFCv4 06/30] Bluetooth: Timers fixes Andrei Emeltchenko
2012-03-20  3:10   ` Gustavo Padovan
2012-03-20 12:21   ` Ulisses Furquim
2012-03-20 13:00     ` Andrei Emeltchenko
2012-03-15 12:29 ` [RFCv4 07/30] Bluetooth: A2MP: Create A2MP channel Andrei Emeltchenko
2012-03-15 12:29 ` [RFCv4 08/30] Bluetooth: A2MP: AMP Manager basic functions Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 09/30] Bluetooth: A2MP: Add channel close callback Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 10/30] Bluetooth: Add state_change for A2MP channel Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 11/30] Bluetooth: A2MP: Build and Send msg helpers Andrei Emeltchenko
2012-03-17 23:12   ` Gustavo Padovan
2012-03-15 12:30 ` [RFCv4 12/30] Bluetooth: A2MP: skb allocation callback Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 13/30] Bluetooth: A2MP: Definitions for A2MP commands Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 14/30] Bluetooth: A2MP: Define A2MP status codes Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 15/30] Bluetooth: A2MP: Process A2MP messages Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 16/30] Bluetooth: A2MP: Process A2MP Command Reject Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 17/30] Bluetooth: A2MP: Helper functions to count HCI devs Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 18/30] Bluetooth: A2MP: Process A2MP Discover Request Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 19/30] Bluetooth: A2MP: Process A2MP Change Notify Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 20/30] Bluetooth: A2MP: Process A2MP Get Info Request Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 21/30] Bluetooth: A2MP: Process A2MP Get AMP Assoc Request Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 22/30] Bluetooth: A2MP: Process A2MP Create Physlink Request Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 23/30] Bluetooth: A2MP: Process A2MP Disc " Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 24/30] Bluetooth: A2MP: Process A2MP Command Responses Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 25/30] Bluetooth: A2MP: Handling fixed channels Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 26/30] Bluetooth: A2MP: Manage incoming connections Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 27/30] Bluetooth: physical link HCI interface to AMP Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 28/30] Bluetooth: Define AMP controller statuses Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 29/30] Bluetooth: General HCI callback implementation Andrei Emeltchenko
2012-03-15 12:30 ` [RFCv4 30/30] Bluetooth: Process HCI callbacks in a workqueue Andrei Emeltchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120320124925.GE27063@aemeltch-MOBL1 \
    --to=andrei.emeltchenko.news@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=ulisses@profusion.mobi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox