Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220224100641.2449550-1-gavin@matician.com> <71D25C8F-67D1-4EC0-9160-5F61C832F0AF@holtmann.org>
In-Reply-To: <71D25C8F-67D1-4EC0-9160-5F61C832F0AF@holtmann.org>
From:   Gavin Li <gavin@matician.com>
Date:   Thu, 24 Feb 2022 12:56:39 -0800
Message-ID: <CACB127+s8mj2-Hkt4yZXRZ_cQ9CbAK0r3y709-8Cb3awpVJm9Q@mail.gmail.com>
Subject: Re: [PATCH] Bluetooth: fix incorrect nonblock bitmask in bt_sock_wait_ready()
To:     Marcel Holtmann <marcel@holtmann.org>
Cc:     Johan Hedberg <johan.hedberg@gmail.com>,
        Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
        linux-bluetooth@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Marcel,

Thanks for reviewing this quickly.

> > diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> > index ee319779781e6..69374321130e4 100644
> > --- a/net/bluetooth/af_bluetooth.c
> > +++ b/net/bluetooth/af_bluetooth.c
> > @@ -568,7 +568,7 @@ int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
> > EXPORT_SYMBOL(bt_sock_wait_state);
> >
> > /* This function expects the sk lock to be held when called */
> > -int bt_sock_wait_ready(struct sock *sk, unsigned long flags)
> > +int bt_sock_wait_ready(struct sock *sk, unsigned int flags)
>
> can we then also do s/flags/msg_flags/ then.
I prefer keeping it as flags because all other net code also uses
flags, msg_flags only appears
in msg->msg_flags.

> > @@ -576,7 +576,7 @@ int bt_sock_wait_ready(struct sock *sk, unsigned long flags)
> >
> >       BT_DBG("sk %p", sk);
> >
> > -     timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
> > +     timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
>
> Since sock_sndtimeo() is taking a bool. This might be better !!(flags & MSG_DONTWAIT).
It appears to be well-known in the net code that sock_sndtimeo takes a
bool, since no other
uses of it do the "!!" conversion.

Let me know what you think. I can make the changes if needed but I was
just trying my best
to match the currently existing convention.

Best,
Gavin