Re: [ovs-dev] [PATCH] db-ctl-base: Fix memory leak of db commands.

2023-10-18 Thread Ilya Maximets
On 10/18/23 12:09, Simon Horman wrote:
> On Tue, Oct 17, 2023 at 07:11:19PM +0800, Zengyuan Wang via dev wrote:
>> Variable "want_key" in function check_condition and variable "key" in 
>> function set_column were not destroyed in exception branch.
>>
>> This patch calls ovsdb_atom_destroy to release resources to avoid memory 
>> leak.
>>
>> Fixes: 79c1a00fb5a5 ("db-ctl-base: Don't die in set_column() on error.")
>> Fixes: e09b3af3e249 ("db-ctl-base: Don't die in is_condition_satisfied() on 
>> error")
>> Signed-off-by: Zengyuan Wang 
> 
> Thanks,
> 
> I agree that there is a possible resource leak here.
> 
> I also agree that the problems seem to have been introduced
> by the cited commits.
> 
> And I see that both of the cited patches were included in v2.10,
> so any backporting can likely involve both fixes - else it might
> have been better to split this into two patches.g
> 
> Overall, this looks good to me.
> 
> Acked-by: Simon Horman 

Thanks!  Applied and backported down to 2.17.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] db-ctl-base: Fix memory leak of db commands.

2023-10-18 Thread Simon Horman
On Tue, Oct 17, 2023 at 07:11:19PM +0800, Zengyuan Wang via dev wrote:
> Variable "want_key" in function check_condition and variable "key" in 
> function set_column were not destroyed in exception branch.
> 
> This patch calls ovsdb_atom_destroy to release resources to avoid memory leak.
> 
> Fixes: 79c1a00fb5a5 ("db-ctl-base: Don't die in set_column() on error.")
> Fixes: e09b3af3e249 ("db-ctl-base: Don't die in is_condition_satisfied() on 
> error")
> Signed-off-by: Zengyuan Wang 

Thanks,

I agree that there is a possible resource leak here.

I also agree that the problems seem to have been introduced
by the cited commits.

And I see that both of the cited patches were included in v2.10,
so any backporting can likely involve both fixes - else it might
have been better to split this into two patches.g

Overall, this looks good to me.

Acked-by: Simon Horman 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] db-ctl-base: Fix memory leak of db commands.

2023-10-17 Thread Zengyuan Wang via dev
Variable "want_key" in function check_condition and variable "key" in function 
set_column were not destroyed in exception branch.

This patch calls ovsdb_atom_destroy to release resources to avoid memory leak.

Fixes: 79c1a00fb5a5 ("db-ctl-base: Don't die in set_column() on error.")
Fixes: e09b3af3e249 ("db-ctl-base: Don't die in is_condition_satisfied() on 
error")
Signed-off-by: Zengyuan Wang 
---
 lib/db-ctl-base.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index 5d2635946..3a8068b12 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -820,6 +820,7 @@ check_condition(const struct ovsdb_idl_table_class *table,
 type.value.type = OVSDB_TYPE_VOID;
 error = ovsdb_datum_from_string(&b, &type, value_string, symtab);
 if (error) {
+ovsdb_atom_destroy(&want_key, column->type.key.type);
 goto out;
 }
 
@@ -1374,6 +1375,7 @@ set_column(const struct ovsdb_idl_table_class *table,
 error = ovsdb_atom_from_string(&value, NULL, &column->type.value,
value_string, symtab);
 if (error) {
+ovsdb_atom_destroy(&key, column->type.key.type);
 goto out;
 }
 
-- 
2.22.0.windows.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] db-ctl-base: fix memory leak of db commands

2023-10-17 Thread 0-day Robot
Bleep bloop.  Greetings Zengyuan Wang, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: The subject summary should start with a capital.
WARNING: The subject summary should end with a dot.
Subject: db-ctl-base: fix memory leak of db commands
Lines checked: 40, Warnings: 2, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] db-ctl-base: fix memory leak of db commands

2023-10-17 Thread Zengyuan Wang via dev
Variable "want_key" in function check_condition and variable "key" in function 
set_column were not destroyed in exception branch.

This patch calls ovsdb_atom_destroy to release resources to avoid memory leak.

Fixes: 79c1a00fb5a5 ("db-ctl-base: Don't die in set_column() on error.")
Fixes: e09b3af3e249 ("db-ctl-base: Don't die in is_condition_satisfied() on 
error")
Signed-off-by: Zengyuan Wang 
---
 lib/db-ctl-base.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index 5d2635946..3a8068b12 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -820,6 +820,7 @@ check_condition(const struct ovsdb_idl_table_class *table,
 type.value.type = OVSDB_TYPE_VOID;
 error = ovsdb_datum_from_string(&b, &type, value_string, symtab);
 if (error) {
+ovsdb_atom_destroy(&want_key, column->type.key.type);
 goto out;
 }
 
@@ -1374,6 +1375,7 @@ set_column(const struct ovsdb_idl_table_class *table,
 error = ovsdb_atom_from_string(&value, NULL, &column->type.value,
value_string, symtab);
 if (error) {
+ovsdb_atom_destroy(&key, column->type.key.type);
 goto out;
 }
 
-- 
2.22.0.windows.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev