All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -next] bpf: remove set but not used variable 'dst_known'
@ 2020-04-13 11:37 ` Mao Wenan
  0 siblings, 0 replies; 16+ messages in thread
From: Mao Wenan @ 2020-04-13 11:37 UTC (permalink / raw)
  To: ast, daniel, kafai, songliubraving, yhs, andriin, john.fastabend,
	kpsingh
  Cc: netdev, bpf, linux-kernel, kernel-janitors

Fixes gcc '-Wunused-but-set-variable' warning:

kernel/bpf/verifier.c:5603:18: warning: variable ‘dst_known’
set but not used [-Wunused-but-set-variable]

It is not used since commit f1174f77b50c ("bpf/verifier:
rework value tracking")

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 kernel/bpf/verifier.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 04c6630cc18f..c9f50969a689 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5600,7 +5600,7 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
 {
 	struct bpf_reg_state *regs = cur_regs(env);
 	u8 opcode = BPF_OP(insn->code);
-	bool src_known, dst_known;
+	bool src_known;
 	s64 smin_val, smax_val;
 	u64 umin_val, umax_val;
 	s32 s32_min_val, s32_max_val;
@@ -5622,7 +5622,6 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
 
 	if (alu32) {
 		src_known = tnum_subreg_is_const(src_reg.var_off);
-		dst_known = tnum_subreg_is_const(dst_reg->var_off);
 		if ((src_known &&
 		     (s32_min_val != s32_max_val || u32_min_val != u32_max_val)) ||
 		    s32_min_val > s32_max_val || u32_min_val > u32_max_val) {
@@ -5634,7 +5633,6 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
 		}
 	} else {
 		src_known = tnum_is_const(src_reg.var_off);
-		dst_known = tnum_is_const(dst_reg->var_off);
 		if ((src_known &&
 		     (smin_val != smax_val || umin_val != umax_val)) ||
 		    smin_val > smax_val || umin_val > umax_val) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH -next] bpf: remove set but not used variable 'dst_known'
@ 2020-04-13 11:37 ` Mao Wenan
  0 siblings, 0 replies; 16+ messages in thread
From: Mao Wenan @ 2020-04-13 11:37 UTC (permalink / raw)
  To: ast, daniel, kafai, songliubraving, yhs, andriin, john.fastabend,
	kpsingh
  Cc: netdev, bpf, linux-kernel, kernel-janitors

Fixes gcc '-Wunused-but-set-variable' warning:

kernel/bpf/verifier.c:5603:18: warning: variable ‘dst_known’
set but not used [-Wunused-but-set-variable]

It is not used since commit f1174f77b50c ("bpf/verifier:
rework value tracking")

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 kernel/bpf/verifier.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 04c6630cc18f..c9f50969a689 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5600,7 +5600,7 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
 {
 	struct bpf_reg_state *regs = cur_regs(env);
 	u8 opcode = BPF_OP(insn->code);
-	bool src_known, dst_known;
+	bool src_known;
 	s64 smin_val, smax_val;
 	u64 umin_val, umax_val;
 	s32 s32_min_val, s32_max_val;
@@ -5622,7 +5622,6 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
 
 	if (alu32) {
 		src_known = tnum_subreg_is_const(src_reg.var_off);
-		dst_known = tnum_subreg_is_const(dst_reg->var_off);
 		if ((src_known &&
 		     (s32_min_val != s32_max_val || u32_min_val != u32_max_val)) ||
 		    s32_min_val > s32_max_val || u32_min_val > u32_max_val) {
@@ -5634,7 +5633,6 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
 		}
 	} else {
 		src_known = tnum_is_const(src_reg.var_off);
-		dst_known = tnum_is_const(dst_reg->var_off);
 		if ((src_known &&
 		     (smin_val != smax_val || umin_val != umax_val)) ||
 		    smin_val > smax_val || umin_val > umax_val) {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* Re: [PATCH -next] bpf: remove set but not used variable 'dst_known'
  2020-04-13 11:37 ` Mao Wenan
@ 2020-04-14 22:05   ` Song Liu
  -1 siblings, 0 replies; 16+ messages in thread
From: Song Liu @ 2020-04-14 22:05 UTC (permalink / raw)
  To: Mao Wenan
  Cc: Alexei Starovoitov, Daniel Borkmann, Martin Lau, Yonghong Song,
	Andrii Nakryiko, john.fastabend, kpsingh, netdev, bpf,
	linux-kernel, kernel-janitors



> On Apr 13, 2020, at 4:37 AM, Mao Wenan <maowenan@huawei.com> wrote:
> 
> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> kernel/bpf/verifier.c:5603:18: warning: variable ‘dst_known’
> set but not used [-Wunused-but-set-variable]
> 
> It is not used since commit f1174f77b50c ("bpf/verifier:
> rework value tracking")

The fix makes sense. But I think f1174f77b50c introduced dst_known, 
so this statement is not accurate. 

> 
> Signed-off-by: Mao Wenan <maowenan@huawei.com>


^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH -next] bpf: remove set but not used variable 'dst_known'
@ 2020-04-14 22:05   ` Song Liu
  0 siblings, 0 replies; 16+ messages in thread
From: Song Liu @ 2020-04-14 22:05 UTC (permalink / raw)
  To: Mao Wenan
  Cc: Alexei Starovoitov, Daniel Borkmann, Martin Lau, Yonghong Song,
	Andrii Nakryiko, john.fastabend, kpsingh, netdev, bpf,
	linux-kernel, kernel-janitors

DQoNCj4gT24gQXByIDEzLCAyMDIwLCBhdCA0OjM3IEFNLCBNYW8gV2VuYW4gPG1hb3dlbmFuQGh1
YXdlaS5jb20+IHdyb3RlOg0KPiANCj4gRml4ZXMgZ2NjICctV3VudXNlZC1idXQtc2V0LXZhcmlh
YmxlJyB3YXJuaW5nOg0KPiANCj4ga2VybmVsL2JwZi92ZXJpZmllci5jOjU2MDM6MTg6IHdhcm5p
bmc6IHZhcmlhYmxlIOKAmGRzdF9rbm93buKAmQ0KPiBzZXQgYnV0IG5vdCB1c2VkIFstV3VudXNl
ZC1idXQtc2V0LXZhcmlhYmxlXQ0KPiANCj4gSXQgaXMgbm90IHVzZWQgc2luY2UgY29tbWl0IGYx
MTc0Zjc3YjUwYyAoImJwZi92ZXJpZmllcjoNCj4gcmV3b3JrIHZhbHVlIHRyYWNraW5nIikNCg0K
VGhlIGZpeCBtYWtlcyBzZW5zZS4gQnV0IEkgdGhpbmsgZjExNzRmNzdiNTBjIGludHJvZHVjZWQg
ZHN0X2tub3duLCANCnNvIHRoaXMgc3RhdGVtZW50IGlzIG5vdCBhY2N1cmF0ZS4gDQoNCj4gDQo+
IFNpZ25lZC1vZmYtYnk6IE1hbyBXZW5hbiA8bWFvd2VuYW5AaHVhd2VpLmNvbT4NCg0K

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH -next] bpf: remove set but not used variable 'dst_known'
  2020-04-14 22:05   ` Song Liu
@ 2020-04-15  1:37     ` maowenan
  -1 siblings, 0 replies; 16+ messages in thread
From: maowenan @ 2020-04-15  1:37 UTC (permalink / raw)
  To: Song Liu
  Cc: Alexei Starovoitov, Daniel Borkmann, Martin Lau, Yonghong Song,
	Andrii Nakryiko, john.fastabend, kpsingh, netdev, bpf,
	linux-kernel, kernel-janitors

On 2020/4/15 6:05, Song Liu wrote:
> 
> 
>> On Apr 13, 2020, at 4:37 AM, Mao Wenan <maowenan@huawei.com> wrote:
>>
>> Fixes gcc '-Wunused-but-set-variable' warning:
>>
>> kernel/bpf/verifier.c:5603:18: warning: variable ‘dst_known’
>> set but not used [-Wunused-but-set-variable]
>>
>> It is not used since commit f1174f77b50c ("bpf/verifier:
>> rework value tracking")
> 
> The fix makes sense. But I think f1174f77b50c introduced dst_known, 
> so this statement is not accurate. 
> 
thanks for review, yes, f1174f77b50c introduced dst_known, and below commit
doesn't deference variable dst_known. So I send v2 later?
3f50f132d840 ("bpf: Verifier, do explicit ALU32 bounds tracking")

>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH -next] bpf: remove set but not used variable 'dst_known'
@ 2020-04-15  1:37     ` maowenan
  0 siblings, 0 replies; 16+ messages in thread
From: maowenan @ 2020-04-15  1:37 UTC (permalink / raw)
  To: Song Liu
  Cc: Alexei Starovoitov, Daniel Borkmann, Martin Lau, Yonghong Song,
	Andrii Nakryiko, john.fastabend, kpsingh, netdev, bpf,
	linux-kernel, kernel-janitors

On 2020/4/15 6:05, Song Liu wrote:
> 
> 
>> On Apr 13, 2020, at 4:37 AM, Mao Wenan <maowenan@huawei.com> wrote:
>>
>> Fixes gcc '-Wunused-but-set-variable' warning:
>>
>> kernel/bpf/verifier.c:5603:18: warning: variable ‘dst_known’
>> set but not used [-Wunused-but-set-variable]
>>
>> It is not used since commit f1174f77b50c ("bpf/verifier:
>> rework value tracking")
> 
> The fix makes sense. But I think f1174f77b50c introduced dst_known, 
> so this statement is not accurate. 
> 
thanks for review, yes, f1174f77b50c introduced dst_known, and below commit
doesn't deference variable dst_known. So I send v2 later?
3f50f132d840 ("bpf: Verifier, do explicit ALU32 bounds tracking")

>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH -next] bpf: remove set but not used variable 'dst_known'
  2020-04-15  1:37     ` maowenan
@ 2020-04-15  7:23       ` Song Liu
  -1 siblings, 0 replies; 16+ messages in thread
From: Song Liu @ 2020-04-15  7:23 UTC (permalink / raw)
  To: maowenan
  Cc: Alexei Starovoitov, Daniel Borkmann, Martin Lau, Yonghong Song,
	Andrii Nakryiko, john.fastabend, kpsingh, netdev, bpf,
	linux-kernel, kernel-janitors



> On Apr 14, 2020, at 6:37 PM, maowenan <maowenan@huawei.com> wrote:
> 
> On 2020/4/15 6:05, Song Liu wrote:
>> 
>> 
>>> On Apr 13, 2020, at 4:37 AM, Mao Wenan <maowenan@huawei.com> wrote:
>>> 
>>> Fixes gcc '-Wunused-but-set-variable' warning:
>>> 
>>> kernel/bpf/verifier.c:5603:18: warning: variable ‘dst_known’
>>> set but not used [-Wunused-but-set-variable]
>>> 
>>> It is not used since commit f1174f77b50c ("bpf/verifier:
>>> rework value tracking")
>> 
>> The fix makes sense. But I think f1174f77b50c introduced dst_known, 
>> so this statement is not accurate. 
>> 
> thanks for review, yes, f1174f77b50c introduced dst_known, and below commit
> doesn't deference variable dst_known. So I send v2 later?
> 3f50f132d840 ("bpf: Verifier, do explicit ALU32 bounds tracking")

I don't think we need to back port this to stable. So it is OK not to 
include Fixes tag. We can just remove this statement in the commit log.

bpf-next is not open yet. Please send v2 when bpf-next is open. 

Thanks,
Song


^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH -next] bpf: remove set but not used variable 'dst_known'
@ 2020-04-15  7:23       ` Song Liu
  0 siblings, 0 replies; 16+ messages in thread
From: Song Liu @ 2020-04-15  7:23 UTC (permalink / raw)
  To: maowenan
  Cc: Alexei Starovoitov, Daniel Borkmann, Martin Lau, Yonghong Song,
	Andrii Nakryiko, john.fastabend, kpsingh, netdev, bpf,
	linux-kernel, kernel-janitors

DQoNCj4gT24gQXByIDE0LCAyMDIwLCBhdCA2OjM3IFBNLCBtYW93ZW5hbiA8bWFvd2VuYW5AaHVh
d2VpLmNvbT4gd3JvdGU6DQo+IA0KPiBPbiAyMDIwLzQvMTUgNjowNSwgU29uZyBMaXUgd3JvdGU6
DQo+PiANCj4+IA0KPj4+IE9uIEFwciAxMywgMjAyMCwgYXQgNDozNyBBTSwgTWFvIFdlbmFuIDxt
YW93ZW5hbkBodWF3ZWkuY29tPiB3cm90ZToNCj4+PiANCj4+PiBGaXhlcyBnY2MgJy1XdW51c2Vk
LWJ1dC1zZXQtdmFyaWFibGUnIHdhcm5pbmc6DQo+Pj4gDQo+Pj4ga2VybmVsL2JwZi92ZXJpZmll
ci5jOjU2MDM6MTg6IHdhcm5pbmc6IHZhcmlhYmxlIOKAmGRzdF9rbm93buKAmQ0KPj4+IHNldCBi
dXQgbm90IHVzZWQgWy1XdW51c2VkLWJ1dC1zZXQtdmFyaWFibGVdDQo+Pj4gDQo+Pj4gSXQgaXMg
bm90IHVzZWQgc2luY2UgY29tbWl0IGYxMTc0Zjc3YjUwYyAoImJwZi92ZXJpZmllcjoNCj4+PiBy
ZXdvcmsgdmFsdWUgdHJhY2tpbmciKQ0KPj4gDQo+PiBUaGUgZml4IG1ha2VzIHNlbnNlLiBCdXQg
SSB0aGluayBmMTE3NGY3N2I1MGMgaW50cm9kdWNlZCBkc3Rfa25vd24sIA0KPj4gc28gdGhpcyBz
dGF0ZW1lbnQgaXMgbm90IGFjY3VyYXRlLiANCj4+IA0KPiB0aGFua3MgZm9yIHJldmlldywgeWVz
LCBmMTE3NGY3N2I1MGMgaW50cm9kdWNlZCBkc3Rfa25vd24sIGFuZCBiZWxvdyBjb21taXQNCj4g
ZG9lc24ndCBkZWZlcmVuY2UgdmFyaWFibGUgZHN0X2tub3duLiBTbyBJIHNlbmQgdjIgbGF0ZXI/
DQo+IDNmNTBmMTMyZDg0MCAoImJwZjogVmVyaWZpZXIsIGRvIGV4cGxpY2l0IEFMVTMyIGJvdW5k
cyB0cmFja2luZyIpDQoNCkkgZG9uJ3QgdGhpbmsgd2UgbmVlZCB0byBiYWNrIHBvcnQgdGhpcyB0
byBzdGFibGUuIFNvIGl0IGlzIE9LIG5vdCB0byANCmluY2x1ZGUgRml4ZXMgdGFnLiBXZSBjYW4g
anVzdCByZW1vdmUgdGhpcyBzdGF0ZW1lbnQgaW4gdGhlIGNvbW1pdCBsb2cuDQoNCmJwZi1uZXh0
IGlzIG5vdCBvcGVuIHlldC4gUGxlYXNlIHNlbmQgdjIgd2hlbiBicGYtbmV4dCBpcyBvcGVuLiAN
Cg0KVGhhbmtzLA0KU29uZw0KDQo

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH -next] bpf: remove set but not used variable 'dst_known'
  2020-04-15  7:23       ` Song Liu
@ 2020-04-15  7:52         ` maowenan
  -1 siblings, 0 replies; 16+ messages in thread
From: maowenan @ 2020-04-15  7:52 UTC (permalink / raw)
  To: Song Liu
  Cc: Alexei Starovoitov, Daniel Borkmann, Martin Lau, Yonghong Song,
	Andrii Nakryiko, john.fastabend, kpsingh, netdev, bpf,
	linux-kernel, kernel-janitors

On 2020/4/15 15:23, Song Liu wrote:
> 
> 
>> On Apr 14, 2020, at 6:37 PM, maowenan <maowenan@huawei.com> wrote:
>>
>> On 2020/4/15 6:05, Song Liu wrote:
>>>
>>>
>>>> On Apr 13, 2020, at 4:37 AM, Mao Wenan <maowenan@huawei.com> wrote:
>>>>
>>>> Fixes gcc '-Wunused-but-set-variable' warning:
>>>>
>>>> kernel/bpf/verifier.c:5603:18: warning: variable ‘dst_known’
>>>> set but not used [-Wunused-but-set-variable]
>>>>
>>>> It is not used since commit f1174f77b50c ("bpf/verifier:
>>>> rework value tracking")
>>>
>>> The fix makes sense. But I think f1174f77b50c introduced dst_known, 
>>> so this statement is not accurate. 
>>>
>> thanks for review, yes, f1174f77b50c introduced dst_known, and below commit
>> doesn't deference variable dst_known. So I send v2 later?
>> 3f50f132d840 ("bpf: Verifier, do explicit ALU32 bounds tracking")
> 
> I don't think we need to back port this to stable. So it is OK not to 
> include Fixes tag. We can just remove this statement in the commit log.
> 
> bpf-next is not open yet. Please send v2 when bpf-next is open. 
> 
> Thanks,
> Song
> 
OK, I will do that.


^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH -next] bpf: remove set but not used variable 'dst_known'
@ 2020-04-15  7:52         ` maowenan
  0 siblings, 0 replies; 16+ messages in thread
From: maowenan @ 2020-04-15  7:52 UTC (permalink / raw)
  To: Song Liu
  Cc: Alexei Starovoitov, Daniel Borkmann, Martin Lau, Yonghong Song,
	Andrii Nakryiko, john.fastabend, kpsingh, netdev, bpf,
	linux-kernel, kernel-janitors

On 2020/4/15 15:23, Song Liu wrote:
> 
> 
>> On Apr 14, 2020, at 6:37 PM, maowenan <maowenan@huawei.com> wrote:
>>
>> On 2020/4/15 6:05, Song Liu wrote:
>>>
>>>
>>>> On Apr 13, 2020, at 4:37 AM, Mao Wenan <maowenan@huawei.com> wrote:
>>>>
>>>> Fixes gcc '-Wunused-but-set-variable' warning:
>>>>
>>>> kernel/bpf/verifier.c:5603:18: warning: variable ‘dst_known’
>>>> set but not used [-Wunused-but-set-variable]
>>>>
>>>> It is not used since commit f1174f77b50c ("bpf/verifier:
>>>> rework value tracking")
>>>
>>> The fix makes sense. But I think f1174f77b50c introduced dst_known, 
>>> so this statement is not accurate. 
>>>
>> thanks for review, yes, f1174f77b50c introduced dst_known, and below commit
>> doesn't deference variable dst_known. So I send v2 later?
>> 3f50f132d840 ("bpf: Verifier, do explicit ALU32 bounds tracking")
> 
> I don't think we need to back port this to stable. So it is OK not to 
> include Fixes tag. We can just remove this statement in the commit log.
> 
> bpf-next is not open yet. Please send v2 when bpf-next is open. 
> 
> Thanks,
> Song
> 
OK, I will do that.

^ permalink raw reply	[flat|nested] 16+ messages in thread
* [PATCH bpf-next v2] bpf: remove set but not used variable 'dst_known'
  2020-04-15  7:52         ` maowenan
@ 2020-04-18  1:37           ` Mao Wenan
  -1 siblings, 0 replies; 16+ messages in thread
From: Mao Wenan @ 2020-04-18  1:37 UTC (permalink / raw)
  To: ast, daniel, kafai, songliubraving, yhs, andriin, john.fastabend,
	kpsingh
  Cc: netdev, bpf, linux-kernel, kernel-janitors

Fixes gcc '-Wunused-but-set-variable' warning:

kernel/bpf/verifier.c:5603:18: warning: variable ‘dst_known’
set but not used [-Wunused-but-set-variable], delete this
variable.

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 v2: remove fixes tag in commit log. 
 kernel/bpf/verifier.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 04c6630cc18f..c9f50969a689 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5600,7 +5600,7 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
 {
 	struct bpf_reg_state *regs = cur_regs(env);
 	u8 opcode = BPF_OP(insn->code);
-	bool src_known, dst_known;
+	bool src_known;
 	s64 smin_val, smax_val;
 	u64 umin_val, umax_val;
 	s32 s32_min_val, s32_max_val;
@@ -5622,7 +5622,6 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
 
 	if (alu32) {
 		src_known = tnum_subreg_is_const(src_reg.var_off);
-		dst_known = tnum_subreg_is_const(dst_reg->var_off);
 		if ((src_known &&
 		     (s32_min_val != s32_max_val || u32_min_val != u32_max_val)) ||
 		    s32_min_val > s32_max_val || u32_min_val > u32_max_val) {
@@ -5634,7 +5633,6 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
 		}
 	} else {
 		src_known = tnum_is_const(src_reg.var_off);
-		dst_known = tnum_is_const(dst_reg->var_off);
 		if ((src_known &&
 		     (smin_val != smax_val || umin_val != umax_val)) ||
 		    smin_val > smax_val || umin_val > umax_val) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH bpf-next v2] bpf: remove set but not used variable 'dst_known'
@ 2020-04-18  1:37           ` Mao Wenan
  0 siblings, 0 replies; 16+ messages in thread
From: Mao Wenan @ 2020-04-18  1:37 UTC (permalink / raw)
  To: ast, daniel, kafai, songliubraving, yhs, andriin, john.fastabend,
	kpsingh
  Cc: netdev, bpf, linux-kernel, kernel-janitors

Fixes gcc '-Wunused-but-set-variable' warning:

kernel/bpf/verifier.c:5603:18: warning: variable ‘dst_known’
set but not used [-Wunused-but-set-variable], delete this
variable.

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 v2: remove fixes tag in commit log. 
 kernel/bpf/verifier.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 04c6630cc18f..c9f50969a689 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5600,7 +5600,7 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
 {
 	struct bpf_reg_state *regs = cur_regs(env);
 	u8 opcode = BPF_OP(insn->code);
-	bool src_known, dst_known;
+	bool src_known;
 	s64 smin_val, smax_val;
 	u64 umin_val, umax_val;
 	s32 s32_min_val, s32_max_val;
@@ -5622,7 +5622,6 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
 
 	if (alu32) {
 		src_known = tnum_subreg_is_const(src_reg.var_off);
-		dst_known = tnum_subreg_is_const(dst_reg->var_off);
 		if ((src_known &&
 		     (s32_min_val != s32_max_val || u32_min_val != u32_max_val)) ||
 		    s32_min_val > s32_max_val || u32_min_val > u32_max_val) {
@@ -5634,7 +5633,6 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
 		}
 	} else {
 		src_known = tnum_is_const(src_reg.var_off);
-		dst_known = tnum_is_const(dst_reg->var_off);
 		if ((src_known &&
 		     (smin_val != smax_val || umin_val != umax_val)) ||
 		    smin_val > smax_val || umin_val > umax_val) {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v2] bpf: remove set but not used variable 'dst_known'
  2020-04-18  1:37           ` Mao Wenan
@ 2020-04-18  6:13             ` Song Liu
  -1 siblings, 0 replies; 16+ messages in thread
From: Song Liu @ 2020-04-18  6:13 UTC (permalink / raw)
  To: Mao Wenan
  Cc: ast, daniel, Martin Lau, Yonghong Song, Andrii Nakryiko,
	john.fastabend, kpsingh, netdev, bpf, linux-kernel,
	kernel-janitors



> On Apr 17, 2020, at 6:37 PM, Mao Wenan <maowenan@huawei.com> wrote:
> 
> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> kernel/bpf/verifier.c:5603:18: warning: variable ‘dst_known’
> set but not used [-Wunused-but-set-variable], delete this
> variable.
> 
> Signed-off-by: Mao Wenan <maowenan@huawei.com>

Acked-by: Song Liu <songliubraving@fb.com>

With one nit below. 

> ---
> v2: remove fixes tag in commit log. 
> kernel/bpf/verifier.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 04c6630cc18f..c9f50969a689 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5600,7 +5600,7 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
> {
> 	struct bpf_reg_state *regs = cur_regs(env);
> 	u8 opcode = BPF_OP(insn->code);
> -	bool src_known, dst_known;
> +	bool src_known;

This is not a hard rule, but we prefer to keep variable definition in 
"reverse Christmas tree" order. Since we are on this function, let's 
reorder these definitions to something like:

        u64 insn_bitness = (BPF_CLASS(insn->code) == BPF_ALU64) ? 64 : 32;
        struct bpf_reg_state *regs = cur_regs(env);
        u8 opcode = BPF_OP(insn->code);
        u32 dst = insn->dst_reg;
        s64 smin_val, smax_val;
        u64 umin_val, umax_val;
        bool src_known;
        int ret;



^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v2] bpf: remove set but not used variable 'dst_known'
@ 2020-04-18  6:13             ` Song Liu
  0 siblings, 0 replies; 16+ messages in thread
From: Song Liu @ 2020-04-18  6:13 UTC (permalink / raw)
  To: Mao Wenan
  Cc: ast, daniel, Martin Lau, Yonghong Song, Andrii Nakryiko,
	john.fastabend, kpsingh, netdev, bpf, linux-kernel,
	kernel-janitors

DQoNCj4gT24gQXByIDE3LCAyMDIwLCBhdCA2OjM3IFBNLCBNYW8gV2VuYW4gPG1hb3dlbmFuQGh1
YXdlaS5jb20+IHdyb3RlOg0KPiANCj4gRml4ZXMgZ2NjICctV3VudXNlZC1idXQtc2V0LXZhcmlh
YmxlJyB3YXJuaW5nOg0KPiANCj4ga2VybmVsL2JwZi92ZXJpZmllci5jOjU2MDM6MTg6IHdhcm5p
bmc6IHZhcmlhYmxlIOKAmGRzdF9rbm93buKAmQ0KPiBzZXQgYnV0IG5vdCB1c2VkIFstV3VudXNl
ZC1idXQtc2V0LXZhcmlhYmxlXSwgZGVsZXRlIHRoaXMNCj4gdmFyaWFibGUuDQo+IA0KPiBTaWdu
ZWQtb2ZmLWJ5OiBNYW8gV2VuYW4gPG1hb3dlbmFuQGh1YXdlaS5jb20+DQoNCkFja2VkLWJ5OiBT
b25nIExpdSA8c29uZ2xpdWJyYXZpbmdAZmIuY29tPg0KDQpXaXRoIG9uZSBuaXQgYmVsb3cuIA0K
DQo+IC0tLQ0KPiB2MjogcmVtb3ZlIGZpeGVzIHRhZyBpbiBjb21taXQgbG9nLiANCj4ga2VybmVs
L2JwZi92ZXJpZmllci5jIHwgNCArLS0tDQo+IDEgZmlsZSBjaGFuZ2VkLCAxIGluc2VydGlvbigr
KSwgMyBkZWxldGlvbnMoLSkNCj4gDQo+IGRpZmYgLS1naXQgYS9rZXJuZWwvYnBmL3ZlcmlmaWVy
LmMgYi9rZXJuZWwvYnBmL3ZlcmlmaWVyLmMNCj4gaW5kZXggMDRjNjYzMGNjMThmLi5jOWY1MDk2
OWE2ODkgMTAwNjQ0DQo+IC0tLSBhL2tlcm5lbC9icGYvdmVyaWZpZXIuYw0KPiArKysgYi9rZXJu
ZWwvYnBmL3ZlcmlmaWVyLmMNCj4gQEAgLTU2MDAsNyArNTYwMCw3IEBAIHN0YXRpYyBpbnQgYWRq
dXN0X3NjYWxhcl9taW5fbWF4X3ZhbHMoc3RydWN0IGJwZl92ZXJpZmllcl9lbnYgKmVudiwNCj4g
ew0KPiAJc3RydWN0IGJwZl9yZWdfc3RhdGUgKnJlZ3MgPSBjdXJfcmVncyhlbnYpOw0KPiAJdTgg
b3Bjb2RlID0gQlBGX09QKGluc24tPmNvZGUpOw0KPiAtCWJvb2wgc3JjX2tub3duLCBkc3Rfa25v
d247DQo+ICsJYm9vbCBzcmNfa25vd247DQoNClRoaXMgaXMgbm90IGEgaGFyZCBydWxlLCBidXQg
d2UgcHJlZmVyIHRvIGtlZXAgdmFyaWFibGUgZGVmaW5pdGlvbiBpbiANCiJyZXZlcnNlIENocmlz
dG1hcyB0cmVlIiBvcmRlci4gU2luY2Ugd2UgYXJlIG9uIHRoaXMgZnVuY3Rpb24sIGxldCdzIA0K
cmVvcmRlciB0aGVzZSBkZWZpbml0aW9ucyB0byBzb21ldGhpbmcgbGlrZToNCg0KICAgICAgICB1
NjQgaW5zbl9iaXRuZXNzID0gKEJQRl9DTEFTUyhpbnNuLT5jb2RlKSA9PSBCUEZfQUxVNjQpID8g
NjQgOiAzMjsNCiAgICAgICAgc3RydWN0IGJwZl9yZWdfc3RhdGUgKnJlZ3MgPSBjdXJfcmVncyhl
bnYpOw0KICAgICAgICB1OCBvcGNvZGUgPSBCUEZfT1AoaW5zbi0+Y29kZSk7DQogICAgICAgIHUz
MiBkc3QgPSBpbnNuLT5kc3RfcmVnOw0KICAgICAgICBzNjQgc21pbl92YWwsIHNtYXhfdmFsOw0K
ICAgICAgICB1NjQgdW1pbl92YWwsIHVtYXhfdmFsOw0KICAgICAgICBib29sIHNyY19rbm93bjsN
CiAgICAgICAgaW50IHJldDsNCg0KDQo

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v2] bpf: remove set but not used variable 'dst_known'
  2020-04-18  6:13             ` Song Liu
@ 2020-04-21  3:23               ` Alexei Starovoitov
  -1 siblings, 0 replies; 16+ messages in thread
From: Alexei Starovoitov @ 2020-04-21  3:23 UTC (permalink / raw)
  To: Song Liu
  Cc: Mao Wenan, ast, daniel, Martin Lau, Yonghong Song,
	Andrii Nakryiko, john.fastabend, kpsingh, netdev, bpf,
	linux-kernel, kernel-janitors, kernel-team

On Sat, Apr 18, 2020 at 06:13:48AM +0000, Song Liu wrote:
> 
> 
> > On Apr 17, 2020, at 6:37 PM, Mao Wenan <maowenan@huawei.com> wrote:
> > 
> > Fixes gcc '-Wunused-but-set-variable' warning:
> > 
> > kernel/bpf/verifier.c:5603:18: warning: variable ‘dst_known’
> > set but not used [-Wunused-but-set-variable], delete this
> > variable.
> > 
> > Signed-off-by: Mao Wenan <maowenan@huawei.com>
> 
> Acked-by: Song Liu <songliubraving@fb.com>
> 
> With one nit below. 
> 
> > ---
> > v2: remove fixes tag in commit log. 
> > kernel/bpf/verifier.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 04c6630cc18f..c9f50969a689 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -5600,7 +5600,7 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
> > {
> > 	struct bpf_reg_state *regs = cur_regs(env);
> > 	u8 opcode = BPF_OP(insn->code);
> > -	bool src_known, dst_known;
> > +	bool src_known;
> 
> This is not a hard rule, but we prefer to keep variable definition in 
> "reverse Christmas tree" order. Since we are on this function, let's 
> reorder these definitions to something like:
> 
>         u64 insn_bitness = (BPF_CLASS(insn->code) == BPF_ALU64) ? 64 : 32;
>         struct bpf_reg_state *regs = cur_regs(env);
>         u8 opcode = BPF_OP(insn->code);
>         u32 dst = insn->dst_reg;
>         s64 smin_val, smax_val;
>         u64 umin_val, umax_val;
>         bool src_known;
>         int ret;

I don't want folks to keep re-sorting variables and making patches difficult
to backport, do git blame, causing bpf vs bpf-next conflicts, etc.

reverse xmas tree is not mandatory. It's a style preference.
I personally do it for new code, but very rarely for fixes.
And certainly not for this kind of cleanup.

Applied. Thanks

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v2] bpf: remove set but not used variable 'dst_known'
@ 2020-04-21  3:23               ` Alexei Starovoitov
  0 siblings, 0 replies; 16+ messages in thread
From: Alexei Starovoitov @ 2020-04-21  3:23 UTC (permalink / raw)
  To: Song Liu
  Cc: Mao Wenan, ast, daniel, Martin Lau, Yonghong Song,
	Andrii Nakryiko, john.fastabend, kpsingh, netdev, bpf,
	linux-kernel, kernel-janitors, kernel-team

On Sat, Apr 18, 2020 at 06:13:48AM +0000, Song Liu wrote:
> 
> 
> > On Apr 17, 2020, at 6:37 PM, Mao Wenan <maowenan@huawei.com> wrote:
> > 
> > Fixes gcc '-Wunused-but-set-variable' warning:
> > 
> > kernel/bpf/verifier.c:5603:18: warning: variable ‘dst_known’
> > set but not used [-Wunused-but-set-variable], delete this
> > variable.
> > 
> > Signed-off-by: Mao Wenan <maowenan@huawei.com>
> 
> Acked-by: Song Liu <songliubraving@fb.com>
> 
> With one nit below. 
> 
> > ---
> > v2: remove fixes tag in commit log. 
> > kernel/bpf/verifier.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 04c6630cc18f..c9f50969a689 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -5600,7 +5600,7 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
> > {
> > 	struct bpf_reg_state *regs = cur_regs(env);
> > 	u8 opcode = BPF_OP(insn->code);
> > -	bool src_known, dst_known;
> > +	bool src_known;
> 
> This is not a hard rule, but we prefer to keep variable definition in 
> "reverse Christmas tree" order. Since we are on this function, let's 
> reorder these definitions to something like:
> 
>         u64 insn_bitness = (BPF_CLASS(insn->code) = BPF_ALU64) ? 64 : 32;
>         struct bpf_reg_state *regs = cur_regs(env);
>         u8 opcode = BPF_OP(insn->code);
>         u32 dst = insn->dst_reg;
>         s64 smin_val, smax_val;
>         u64 umin_val, umax_val;
>         bool src_known;
>         int ret;

I don't want folks to keep re-sorting variables and making patches difficult
to backport, do git blame, causing bpf vs bpf-next conflicts, etc.

reverse xmas tree is not mandatory. It's a style preference.
I personally do it for new code, but very rarely for fixes.
And certainly not for this kind of cleanup.

Applied. Thanks

^ permalink raw reply	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-04-21  3:23 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-13 11:37 [PATCH -next] bpf: remove set but not used variable 'dst_known' Mao Wenan
2020-04-13 11:37 ` Mao Wenan
2020-04-14 22:05 ` Song Liu
2020-04-14 22:05   ` Song Liu
2020-04-15  1:37   ` maowenan
2020-04-15  1:37     ` maowenan
2020-04-15  7:23     ` Song Liu
2020-04-15  7:23       ` Song Liu
2020-04-15  7:52       ` maowenan
2020-04-15  7:52         ` maowenan
2020-04-18  1:37         ` [PATCH bpf-next v2] " Mao Wenan
2020-04-18  1:37           ` Mao Wenan
2020-04-18  6:13           ` Song Liu
2020-04-18  6:13             ` Song Liu
2020-04-21  3:23             ` Alexei Starovoitov
2020-04-21  3:23               ` Alexei Starovoitov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.