From: Marcel Holtmann <marcel@holtmann.org>
To: Gustavo Padovan <gustavo@padovan.org>
Cc: linux-bluetooth@vger.kernel.org,
Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Subject: Re: [PATCH 03/16] Bluetooth: extend state_change() call to report errors too
Date: Wed, 20 Feb 2013 22:16:17 +0100 [thread overview]
Message-ID: <1361394977.1583.80.camel@aeonflux> (raw)
In-Reply-To: <1361391103-30738-4-git-send-email-gustavo@padovan.org>
Hi Gustavo,
> Now l2cap_core doesn't need to touch sk_err element anymore, it just tell
> l2cap_sock via this call which error it wants to set.
I need an explanation here since the code and the commit message are
unclear to me.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
> include/net/bluetooth/l2cap.h | 2 +-
> net/bluetooth/a2mp.c | 2 +-
> net/bluetooth/l2cap_core.c | 6 ++----
> net/bluetooth/l2cap_sock.c | 6 +++++-
> 4 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index cf94e68..4f28a8c 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -550,7 +550,7 @@ struct l2cap_ops {
> void (*teardown) (struct l2cap_chan *chan, int err);
> void (*close) (struct l2cap_chan *chan);
> void (*state_change) (struct l2cap_chan *chan,
> - int state);
> + int state, int err);
Why is this a good idea? Why not a separate error callback. You need to
explain this.
> void (*ready) (struct l2cap_chan *chan);
> void (*defer) (struct l2cap_chan *chan);
> struct sk_buff *(*alloc_skb) (struct l2cap_chan *chan,
> diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
> index eb0f4b1..ad6e42f 100644
> --- a/net/bluetooth/a2mp.c
> +++ b/net/bluetooth/a2mp.c
> @@ -671,7 +671,7 @@ static void a2mp_chan_close_cb(struct l2cap_chan *chan)
> l2cap_chan_put(chan);
> }
>
> -static void a2mp_chan_state_change_cb(struct l2cap_chan *chan, int state)
> +static void a2mp_chan_state_change_cb(struct l2cap_chan *chan, int state, int err)
> {
> struct amp_mgr *mgr = chan->data;
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index a306cc8..775c83f 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -216,7 +216,7 @@ static void __l2cap_state_change(struct l2cap_chan *chan, int state)
> state_to_string(state));
>
> chan->state = state;
> - chan->ops->state_change(chan, state);
> + chan->ops->state_change(chan, state, 0);
> }
>
> static void l2cap_state_change(struct l2cap_chan *chan, int state)
> @@ -230,9 +230,7 @@ static void l2cap_state_change(struct l2cap_chan *chan, int state)
>
> static inline void __l2cap_chan_set_err(struct l2cap_chan *chan, int err)
> {
> - struct sock *sk = chan->sk;
> -
> - sk->sk_err = err;
> + chan->ops->state_change(chan, chan->state, err);
> }
>
> static inline void l2cap_chan_set_err(struct l2cap_chan *chan, int err)
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 221aef9..8adea0f 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -1044,11 +1044,15 @@ static void l2cap_sock_teardown_cb(struct l2cap_chan *chan, int err)
> release_sock(sk);
> }
>
> -static void l2cap_sock_state_change_cb(struct l2cap_chan *chan, int state)
> +static void l2cap_sock_state_change_cb(struct l2cap_chan *chan, int state,
> + int err)
> {
> struct sock *sk = chan->data;
>
> sk->sk_state = state;
> +
> + if (err)
> + sk->sk_err = err;
> }
>
> static struct sk_buff *l2cap_sock_alloc_skb_cb(struct l2cap_chan *chan,
Regards
Marcel
next prev parent reply other threads:[~2013-02-20 21:16 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-20 20:11 [PATCH 00/16] Completely remove socket dependency from l2cap_core.c Gustavo Padovan
2013-02-20 20:11 ` [PATCH 01/16] Bluetooth: Add src and dst info to struct l2cap_chan Gustavo Padovan
2013-02-20 21:12 ` Marcel Holtmann
2013-02-20 20:11 ` [PATCH 02/16] Bluetooth: Remove sk_sndtimeo from l2cap_core.c Gustavo Padovan
2013-02-20 21:14 ` Marcel Holtmann
2013-02-20 20:11 ` [PATCH 03/16] Bluetooth: extend state_change() call to report errors too Gustavo Padovan
2013-02-20 21:16 ` Marcel Holtmann [this message]
2013-02-20 20:11 ` [PATCH 04/16] Bluetooth: add l2cap_state_change_and_error() Gustavo Padovan
2013-02-20 21:17 ` Marcel Holtmann
2013-02-20 20:11 ` [PATCH 05/16] Bluetooth: Add missing braces to an "else if" Gustavo Padovan
2013-02-20 21:18 ` Marcel Holtmann
2013-02-20 20:11 ` [PATCH 06/16] Bluetooth: use l2cap_chan_ready() instead of duplicate code Gustavo Padovan
2013-02-20 21:20 ` Marcel Holtmann
2013-02-20 20:11 ` [PATCH 07/16] Bluetooth: duplicate DEFER_SETUP flag on l2cap_chan Gustavo Padovan
2013-02-20 21:22 ` Marcel Holtmann
2013-02-20 20:11 ` [PATCH 08/16] Bluetooth: Improving locking in l2cap_conn_start() Gustavo Padovan
2013-02-20 21:23 ` Marcel Holtmann
2013-02-20 20:11 ` [PATCH 09/16] Bluetooth: lock socket in defer_cb call Gustavo Padovan
2013-02-20 20:11 ` [PATCH 10/16] Bluetooth: Remove socket lock from state_change() in l2cap_core Gustavo Padovan
2013-02-20 21:26 ` Marcel Holtmann
2013-02-20 20:11 ` [PATCH 11/16] Bluetooth: remove parent socket usage from l2cap_core.c Gustavo Padovan
2013-02-20 21:28 ` Marcel Holtmann
2013-02-20 20:11 ` [PATCH 12/16] Bluetooth: Use abstract chan->data in comparison Gustavo Padovan
2013-02-20 20:11 ` [PATCH 13/16] Bluetooth: Move l2cap_wait_ack() to l2cap_sock.c Gustavo Padovan
2013-02-20 20:11 ` [PATCH 14/16] Bluetooth: Create l2cap->ops->resume() Gustavo Padovan
2013-02-20 20:11 ` [PATCH 15/16] Bluetooth: Create l2cap->ops->set_shutdown() Gustavo Padovan
2013-02-20 20:11 ` [PATCH 16/16] Bluetooth: Remove sk member from struct l2cap_chan Gustavo Padovan
-- strict thread matches above, loose matches on Subject: below --
2013-01-10 7:47 [PATCH 00/16] Completely remove socket dependency from l2cap_core.c Gustavo Padovan
2013-01-10 7:47 ` [PATCH 03/16] Bluetooth: extend state_change() call to report errors too Gustavo Padovan
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=1361394977.1583.80.camel@aeonflux \
--to=marcel@holtmann.org \
--cc=gustavo.padovan@collabora.co.uk \
--cc=gustavo@padovan.org \
--cc=linux-bluetooth@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.