diff mbox series

[1/6] netlink: fix missing destruction of rhash table in error case

Message ID 20220529153456.4183738-2-cgxu519@mykernel.net
State New
Headers show
Series fix a common error of while loop condition in error path | expand

Commit Message

Chengguang Xu May 29, 2022, 3:34 p.m. UTC
Fix missing destruction(when '(--i) == 0') for error case in
netlink_proto_init().

Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
---
 net/netlink/af_netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Paolo Abeni May 31, 2022, 8:43 a.m. UTC | #1
Hello,

On Sun, 2022-05-29 at 23:34 +0800, Chengguang Xu wrote:
> Fix missing destruction(when '(--i) == 0') for error case in
> netlink_proto_init().
> 
> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
> ---
>  net/netlink/af_netlink.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 0cd91f813a3b..bd0b090a378b 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -2887,7 +2887,7 @@ static int __init netlink_proto_init(void)
>  	for (i = 0; i < MAX_LINKS; i++) {
>  		if (rhashtable_init(&nl_table[i].hash,
>  				    &netlink_rhashtable_params) < 0) {
> -			while (--i > 0)
> +			while (--i >= 0)
>  				rhashtable_destroy(&nl_table[i].hash);
>  			kfree(nl_table);
>  			goto panic;

The patch looks correct to me, but it looks like each patch in this
series is targeting a different tree. I suggest to re-send, splitting
the series into individual patches, and sending each of them to the
appropriate tree. You can retain Dan's Review tag.

Thanks,

Paolo
Dan Carpenter May 31, 2022, 11:25 a.m. UTC | #2
On Tue, May 31, 2022 at 10:43:09AM +0200, Paolo Abeni wrote:
> Hello,
> 
> On Sun, 2022-05-29 at 23:34 +0800, Chengguang Xu wrote:
> > Fix missing destruction(when '(--i) == 0') for error case in
> > netlink_proto_init().
> > 
> > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
> > ---
> >  net/netlink/af_netlink.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> > index 0cd91f813a3b..bd0b090a378b 100644
> > --- a/net/netlink/af_netlink.c
> > +++ b/net/netlink/af_netlink.c
> > @@ -2887,7 +2887,7 @@ static int __init netlink_proto_init(void)
> >  	for (i = 0; i < MAX_LINKS; i++) {
> >  		if (rhashtable_init(&nl_table[i].hash,
> >  				    &netlink_rhashtable_params) < 0) {
> > -			while (--i > 0)
> > +			while (--i >= 0)
> >  				rhashtable_destroy(&nl_table[i].hash);
> >  			kfree(nl_table);
> >  			goto panic;
> 
> The patch looks correct to me, but it looks like each patch in this
> series is targeting a different tree. I suggest to re-send, splitting
> the series into individual patches, and sending each of them to the
> appropriate tree. You can retain Dan's Review tag.

Since it looks like you're going to be resending these then could you
add Fixes tags?  Please keep my Review tag.

regards,
dan carpenter
Chengguang Xu June 1, 2022, 7:47 a.m. UTC | #3
在 2022/5/31 19:25, Dan Carpenter 写道:
> On Tue, May 31, 2022 at 10:43:09AM +0200, Paolo Abeni wrote:
>> Hello,
>>
>> On Sun, 2022-05-29 at 23:34 +0800, Chengguang Xu wrote:
>>> Fix missing destruction(when '(--i) == 0') for error case in
>>> netlink_proto_init().
>>>
>>> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
>>> ---
>>>   net/netlink/af_netlink.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
>>> index 0cd91f813a3b..bd0b090a378b 100644
>>> --- a/net/netlink/af_netlink.c
>>> +++ b/net/netlink/af_netlink.c
>>> @@ -2887,7 +2887,7 @@ static int __init netlink_proto_init(void)
>>>   	for (i = 0; i < MAX_LINKS; i++) {
>>>   		if (rhashtable_init(&nl_table[i].hash,
>>>   				    &netlink_rhashtable_params) < 0) {
>>> -			while (--i > 0)
>>> +			while (--i >= 0)
>>>   				rhashtable_destroy(&nl_table[i].hash);
>>>   			kfree(nl_table);
>>>   			goto panic;
>> The patch looks correct to me, but it looks like each patch in this
>> series is targeting a different tree. I suggest to re-send, splitting
>> the series into individual patches, and sending each of them to the
>> appropriate tree. You can retain Dan's Review tag.
> Since it looks like you're going to be resending these then could you
> add Fixes tags?  Please keep my Review tag.
>

OK, no problem.

Thanks,
Chengguang
diff mbox series

Patch

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 0cd91f813a3b..bd0b090a378b 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2887,7 +2887,7 @@  static int __init netlink_proto_init(void)
 	for (i = 0; i < MAX_LINKS; i++) {
 		if (rhashtable_init(&nl_table[i].hash,
 				    &netlink_rhashtable_params) < 0) {
-			while (--i > 0)
+			while (--i >= 0)
 				rhashtable_destroy(&nl_table[i].hash);
 			kfree(nl_table);
 			goto panic;