Hi Andrei,
* Andrei Emeltchenko <andrei.emeltchenko.news@xxxxxxxxx> [2012-05-24 12:35:59 +0300]:
> Hi Gustavo,
>
> On Wed, May 23, 2012 at 10:12:37PM -0300, Gustavo Padovan wrote:
> > void l2cap_chan_close(struct l2cap_chan *chan, int reason)
> > {
> > struct l2cap_conn *conn = chan->conn;
> > @@ -583,12 +549,8 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
> >
> > switch (chan->state) {
> > case BT_LISTEN:
> > - lock_sock(sk);
> > - l2cap_chan_cleanup_listen(sk);
> > -
> > - __l2cap_state_change(chan, BT_CLOSED);
> > - sock_set_flag(sk, SOCK_ZAPPED);
> > - release_sock(sk);
> > + if (chan->ops->finalize)
> > + chan->ops->finalize(chan->data, 0);
>
> Looks like you are handling all cases so this can be put outside of
> switch.
No, I'm not, look to the BT_CONNECTED/BT_CONFIG option.
>
> > break;
> >
> > case BT_CONNECTED:
> > @@ -630,9 +592,8 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
> > break;
> >
> > default:
> > - lock_sock(sk);
> > - sock_set_flag(sk, SOCK_ZAPPED);
> > - release_sock(sk);
> > + if (chan->ops->finalize)
> > + chan->ops->finalize(chan->data, 0);
> > break;
> > }
> > }
> > @@ -3416,7 +3377,9 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
> >
> > /* Check if we already have channel with that dcid */
> > if (__l2cap_get_chan_by_dcid(conn, scid)) {
> > - sock_set_flag(sk, SOCK_ZAPPED);
> > + if (chan->ops->finalize)
> > + chan->ops->finalize(chan->data, 0);
> > +
> > chan->ops->close(chan->data);
> > goto response;
> > }
> > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > index 4d36605..0302cb4 100644
> > --- a/net/bluetooth/l2cap_sock.c
> > +++ b/net/bluetooth/l2cap_sock.c
> > @@ -872,6 +872,25 @@ static int l2cap_sock_release(struct socket *sock)
> > return err;
> > }
> >
> > +static void l2cap_sock_cleanup_listen(struct sock *parent)
> > +{
> > + struct sock *sk;
> > +
> > + BT_DBG("parent %p", parent);
> > +
> > + /* Close not yet accepted channels */
> > + while ((sk = bt_accept_dequeue(parent, NULL))) {
> > + struct l2cap_chan *chan = l2cap_pi(sk)->chan;
> > +
> > + l2cap_chan_lock(chan);
> > + __clear_chan_timer(chan);
> > + l2cap_chan_close(chan, ECONNRESET);
> > + l2cap_chan_unlock(chan);
> > +
> > + l2cap_sock_kill(sk);
> > + }
> > +}
> > +
> > static struct l2cap_chan *l2cap_sock_new_connection_cb(void *data)
> > {
> > struct sock *sk, *parent = data;
> > @@ -931,6 +950,49 @@ static void l2cap_sock_close_cb(void *data)
> > l2cap_sock_kill(sk);
> > }
> >
> > +static void l2cap_sock_finalize_cb(void *data, int err)
> > +{
> > + struct sock *sk = data;
>
> Why we cannot just use l2cap_sock_finalize_cb(struct sock *sk, int err)
> what is the reason for extra assignment?
Because we want to be generic here, we don't know who else will use finalize()
so we need to keep a void * here.
>
> I also think finalize is not good name since it does not mean what is
> actually finalized.
And it is not, I failed to come with a better name here. The initial one was
delete, even worse.
Gustavo
--
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
