From: Gavin Li <[email protected]>
Callers pass msg->msg_flags as flags, which contains MSG_DONTWAIT
instead of O_NONBLOCK.
Signed-off-by: Gavin Li <[email protected]>
---
include/net/bluetooth/bluetooth.h | 2 +-
net/bluetooth/af_bluetooth.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index a647e5fabdbd6..87f0bba39b0f7 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -343,7 +343,7 @@ int bt_sock_stream_recvmsg(struct socket *sock, struct msghdr *msg,
__poll_t bt_sock_poll(struct file *file, struct socket *sock, poll_table *wait);
int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
-int bt_sock_wait_ready(struct sock *sk, unsigned long flags);
+int bt_sock_wait_ready(struct sock *sk, unsigned int flags);
void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh);
void bt_accept_unlink(struct sock *sk);
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)
{
DECLARE_WAITQUEUE(wait, current);
unsigned long timeo;
@@ -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);
add_wait_queue(sk_sleep(sk), &wait);
set_current_state(TASK_INTERRUPTIBLE);
--
2.34.1
Hi Gavin,
> Callers pass msg->msg_flags as flags, which contains MSG_DONTWAIT
> instead of O_NONBLOCK.
>
> Signed-off-by: Gavin Li <[email protected]>
> ---
> include/net/bluetooth/bluetooth.h | 2 +-
> net/bluetooth/af_bluetooth.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index a647e5fabdbd6..87f0bba39b0f7 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -343,7 +343,7 @@ int bt_sock_stream_recvmsg(struct socket *sock, struct msghdr *msg,
> __poll_t bt_sock_poll(struct file *file, struct socket *sock, poll_table *wait);
> int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
> int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
> -int bt_sock_wait_ready(struct sock *sk, unsigned long flags);
> +int bt_sock_wait_ready(struct sock *sk, unsigned int flags);
>
> void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh);
> void bt_accept_unlink(struct sock *sk);
> 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.
> {
> DECLARE_WAITQUEUE(wait, current);
> unsigned long timeo;
> @@ -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).
Regards
Marcel
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
Hi Marcel,
Please let me know what you think with regards to the comments above.
Best,
Gavin
On Thu, Feb 24, 2022 at 12:56 PM Gavin Li <[email protected]> wrote:
>
> 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
Hi Gavin,
>>> 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.
while that might be true, I find it a lot clearer if the variable is 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.
And other code in the kernel makes sure to clearly turn something into a bool. You get 0x00 and 0x40.
Regards
Marcel