* [PATCH] bpftool: optimize if statement code
@ 2024-10-15 11:09 Liu Jing
2024-10-15 12:11 ` Quentin Monnet
0 siblings, 1 reply; 3+ messages in thread
From: Liu Jing @ 2024-10-15 11:09 UTC (permalink / raw)
To: qmo
Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf, linux-kernel,
Liu Jing
Since both conditions are used to check whether len is valid, we can combine the two conditions into a single if statement
Signed-off-by: Liu Jing <liujing@cmss.chinamobile.com>
---
tools/bpf/bpftool/feature.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index 4dbc4fcdf473..0121e0fd6949 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -158,10 +158,9 @@ static int get_vendor_id(int ifindex)
len = read(fd, buf, sizeof(buf));
close(fd);
- if (len < 0)
- return -1;
- if (len >= (ssize_t)sizeof(buf))
+ if ((len < 0) || (len >= (ssize_t)sizeof(buf)))
return -1;
+
buf[len] = '\0';
return strtol(buf, NULL, 0);
--
2.27.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] bpftool: optimize if statement code
2024-10-15 11:09 [PATCH] bpftool: optimize if statement code Liu Jing
@ 2024-10-15 12:11 ` Quentin Monnet
2024-10-15 18:16 ` Martin KaFai Lau
0 siblings, 1 reply; 3+ messages in thread
From: Quentin Monnet @ 2024-10-15 12:11 UTC (permalink / raw)
To: Liu Jing
Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf, linux-kernel
2024-10-15 19:09 UTC+0800 ~ Liu Jing <liujing@cmss.chinamobile.com>
> Since both conditions are used to check whether len is valid, we can combine the two conditions into a single if statement
> Signed-off-by: Liu Jing <liujing@cmss.chinamobile.com>
> ---
> tools/bpf/bpftool/feature.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index 4dbc4fcdf473..0121e0fd6949 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -158,10 +158,9 @@ static int get_vendor_id(int ifindex)
>
> len = read(fd, buf, sizeof(buf));
> close(fd);
> - if (len < 0)
> - return -1;
> - if (len >= (ssize_t)sizeof(buf))
> + if ((len < 0) || (len >= (ssize_t)sizeof(buf)))
> return -1;
> +
> buf[len] = '\0';
>
> return strtol(buf, NULL, 0);
Thanks. I'm not strictly opposed to the change, but it doesn't bring
much value in my opinion. I don't think this will "optimize" the
statement beyond what the compiler does already.
Quentin
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] bpftool: optimize if statement code
2024-10-15 12:11 ` Quentin Monnet
@ 2024-10-15 18:16 ` Martin KaFai Lau
0 siblings, 0 replies; 3+ messages in thread
From: Martin KaFai Lau @ 2024-10-15 18:16 UTC (permalink / raw)
To: Liu Jing
Cc: Quentin Monnet, ast, daniel, andrii, eddyz87, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf, linux-kernel
On 10/15/24 5:11 AM, Quentin Monnet wrote:
> 2024-10-15 19:09 UTC+0800 ~ Liu Jing <liujing@cmss.chinamobile.com>
>> Since both conditions are used to check whether len is valid, we can combine
>> the two conditions into a single if statement
>> Signed-off-by: Liu Jing <liujing@cmss.chinamobile.com>
>> ---
>> tools/bpf/bpftool/feature.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
>> index 4dbc4fcdf473..0121e0fd6949 100644
>> --- a/tools/bpf/bpftool/feature.c
>> +++ b/tools/bpf/bpftool/feature.c
>> @@ -158,10 +158,9 @@ static int get_vendor_id(int ifindex)
>> len = read(fd, buf, sizeof(buf));
>> close(fd);
>> - if (len < 0)
>> - return -1;
>> - if (len >= (ssize_t)sizeof(buf))
>> + if ((len < 0) || (len >= (ssize_t)sizeof(buf)))
>> return -1;
>> +
>> buf[len] = '\0';
>> return strtol(buf, NULL, 0);
>
>
> Thanks. I'm not strictly opposed to the change, but it doesn't bring much value
> in my opinion. I don't think this will "optimize" the statement beyond what the
> compiler does already.
+1
The current code is good as is.
pw-bot: cr
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-10-15 18:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-15 11:09 [PATCH] bpftool: optimize if statement code Liu Jing
2024-10-15 12:11 ` Quentin Monnet
2024-10-15 18:16 ` Martin KaFai Lau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox