* [PATCH] bpf: hide a possibly unused variable
@ 2018-02-20 21:16 Arnd Bergmann
2018-02-20 21:44 ` Daniel Borkmann
0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2018-02-20 21:16 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, David S. Miller
Cc: Arnd Bergmann, John Fastabend, Lawrence Brakmo,
Jesper Dangaard Brouer, Jakub Kicinski, netdev, linux-kernel
The only user of this variable is inside of an #ifdef, causing
a warning without CONFIG_INET:
net/core/filter.c: In function '____bpf_sock_ops_cb_flags_set':
net/core/filter.c:3382:6: error: unused variable 'val' [-Werror=unused-variable]
int val = argval & BPF_SOCK_OPS_ALL_CB_FLAGS;
This adds the same #ifdef around the declaration.
Fixes: b13d88072172 ("bpf: Adds field bpf_sock_ops_cb_flags to tcp_sock")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/core/filter.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/core/filter.c b/net/core/filter.c
index 08ab4c65a998..c3dc6d60b4bb 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3379,7 +3379,9 @@ BPF_CALL_2(bpf_sock_ops_cb_flags_set, struct bpf_sock_ops_kern *, bpf_sock,
int, argval)
{
struct sock *sk = bpf_sock->sk;
+#ifdef CONFIG_INET
int val = argval & BPF_SOCK_OPS_ALL_CB_FLAGS;
+#endif
if (!sk_fullsock(sk))
return -EINVAL;
--
2.9.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] bpf: hide a possibly unused variable
2018-02-20 21:16 [PATCH] bpf: hide a possibly unused variable Arnd Bergmann
@ 2018-02-20 21:44 ` Daniel Borkmann
2018-02-20 22:08 ` Arnd Bergmann
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Borkmann @ 2018-02-20 21:44 UTC (permalink / raw)
To: Arnd Bergmann, Alexei Starovoitov, David S. Miller
Cc: John Fastabend, Lawrence Brakmo, Jesper Dangaard Brouer,
Jakub Kicinski, netdev, linux-kernel
Hi Arnd,
On 02/20/2018 10:16 PM, Arnd Bergmann wrote:
> The only user of this variable is inside of an #ifdef, causing
> a warning without CONFIG_INET:
>
> net/core/filter.c: In function '____bpf_sock_ops_cb_flags_set':
> net/core/filter.c:3382:6: error: unused variable 'val' [-Werror=unused-variable]
> int val = argval & BPF_SOCK_OPS_ALL_CB_FLAGS;
>
> This adds the same #ifdef around the declaration.
>
> Fixes: b13d88072172 ("bpf: Adds field bpf_sock_ops_cb_flags to tcp_sock")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> net/core/filter.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 08ab4c65a998..c3dc6d60b4bb 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3379,7 +3379,9 @@ BPF_CALL_2(bpf_sock_ops_cb_flags_set, struct bpf_sock_ops_kern *, bpf_sock,
> int, argval)
> {
> struct sock *sk = bpf_sock->sk;
> +#ifdef CONFIG_INET
> int val = argval & BPF_SOCK_OPS_ALL_CB_FLAGS;
> +#endif
Looks good, thanks for the fix!
Could you move the existing '#ifdef CONFIG_INET' to the beginning of
the function given the only error in case of !CONFIG_INET is -EINVAL
anyway? That would at least not increase the ifdef ugliness further.
Thanks a lot,
Daniel
> if (!sk_fullsock(sk))
> return -EINVAL;
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] bpf: hide a possibly unused variable
2018-02-20 21:44 ` Daniel Borkmann
@ 2018-02-20 22:08 ` Arnd Bergmann
2018-02-20 22:12 ` Daniel Borkmann
0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2018-02-20 22:08 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, David S. Miller, John Fastabend,
Lawrence Brakmo, Jesper Dangaard Brouer, Jakub Kicinski,
Networking, Linux Kernel Mailing List
On Tue, Feb 20, 2018 at 10:44 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> Hi Arnd,
>
> On 02/20/2018 10:16 PM, Arnd Bergmann wrote:
>> The only user of this variable is inside of an #ifdef, causing
>> a warning without CONFIG_INET:
>>
>> net/core/filter.c: In function '____bpf_sock_ops_cb_flags_set':
>> net/core/filter.c:3382:6: error: unused variable 'val' [-Werror=unused-variable]
>> int val = argval & BPF_SOCK_OPS_ALL_CB_FLAGS;
>>
>> This adds the same #ifdef around the declaration.
>>
>> Fixes: b13d88072172 ("bpf: Adds field bpf_sock_ops_cb_flags to tcp_sock")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> net/core/filter.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 08ab4c65a998..c3dc6d60b4bb 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -3379,7 +3379,9 @@ BPF_CALL_2(bpf_sock_ops_cb_flags_set, struct bpf_sock_ops_kern *, bpf_sock,
>> int, argval)
>> {
>> struct sock *sk = bpf_sock->sk;
>> +#ifdef CONFIG_INET
>> int val = argval & BPF_SOCK_OPS_ALL_CB_FLAGS;
>> +#endif
>
> Looks good, thanks for the fix!
>
> Could you move the existing '#ifdef CONFIG_INET' to the beginning of
> the function given the only error in case of !CONFIG_INET is -EINVAL
> anyway? That would at least not increase the ifdef ugliness further.
Sure, sent a new version now. I decided to clean up that #ifdef
check as well, since a IS_ENABLED() check is nicer anway.
Arnd
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] bpf: hide a possibly unused variable
2018-02-20 22:08 ` Arnd Bergmann
@ 2018-02-20 22:12 ` Daniel Borkmann
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Borkmann @ 2018-02-20 22:12 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Alexei Starovoitov, David S. Miller, John Fastabend,
Lawrence Brakmo, Jesper Dangaard Brouer, Jakub Kicinski,
Networking, Linux Kernel Mailing List
On 02/20/2018 11:08 PM, Arnd Bergmann wrote:
> On Tue, Feb 20, 2018 at 10:44 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
>> Hi Arnd,
>>
>> On 02/20/2018 10:16 PM, Arnd Bergmann wrote:
>>> The only user of this variable is inside of an #ifdef, causing
>>> a warning without CONFIG_INET:
>>>
>>> net/core/filter.c: In function '____bpf_sock_ops_cb_flags_set':
>>> net/core/filter.c:3382:6: error: unused variable 'val' [-Werror=unused-variable]
>>> int val = argval & BPF_SOCK_OPS_ALL_CB_FLAGS;
>>>
>>> This adds the same #ifdef around the declaration.
>>>
>>> Fixes: b13d88072172 ("bpf: Adds field bpf_sock_ops_cb_flags to tcp_sock")
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>> ---
>>> net/core/filter.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/net/core/filter.c b/net/core/filter.c
>>> index 08ab4c65a998..c3dc6d60b4bb 100644
>>> --- a/net/core/filter.c
>>> +++ b/net/core/filter.c
>>> @@ -3379,7 +3379,9 @@ BPF_CALL_2(bpf_sock_ops_cb_flags_set, struct bpf_sock_ops_kern *, bpf_sock,
>>> int, argval)
>>> {
>>> struct sock *sk = bpf_sock->sk;
>>> +#ifdef CONFIG_INET
>>> int val = argval & BPF_SOCK_OPS_ALL_CB_FLAGS;
>>> +#endif
>>
>> Looks good, thanks for the fix!
>>
>> Could you move the existing '#ifdef CONFIG_INET' to the beginning of
>> the function given the only error in case of !CONFIG_INET is -EINVAL
>> anyway? That would at least not increase the ifdef ugliness further.
>
> Sure, sent a new version now. I decided to clean up that #ifdef
> check as well, since a IS_ENABLED() check is nicer anway.
Looks great, thanks! I'll get it into bpf once the pending pr got pulled.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-02-20 22:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-20 21:16 [PATCH] bpf: hide a possibly unused variable Arnd Bergmann
2018-02-20 21:44 ` Daniel Borkmann
2018-02-20 22:08 ` Arnd Bergmann
2018-02-20 22:12 ` Daniel Borkmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox