* [PATCH v2] scsi: fcoe: add missed kfree() in an error path
@ 2020-07-09 12:05 Jing Xiangfeng
2020-07-13 17:53 ` Ewan D. Milne
0 siblings, 1 reply; 3+ messages in thread
From: Jing Xiangfeng @ 2020-07-09 12:05 UTC (permalink / raw)
To: hare, jejb, martin.petersen, robert.w.love, Neerav.Parikh,
Markus.Elfring
Cc: linux-scsi, linux-kernel, jingxiangfeng
fcoe_fdmi_info() misses to call kfree() in an error path.
Add a label 'free_fdmi' and jump to it.
Fixes: f07d46bbc9ba ("fcoe: Fix smatch warning in fcoe_fdmi_info function")
Signed-off-by: Jing Xiangfeng <jingxiangfeng@huawei.com>
---
drivers/scsi/fcoe/fcoe.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 25dae9f0b205..a63057a03772 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -830,7 +830,7 @@ static void fcoe_fdmi_info(struct fc_lport *lport, struct net_device *netdev)
if (rc) {
printk(KERN_INFO "fcoe: Failed to retrieve FDMI "
"information from netdev.\n");
- return;
+ goto free_fdmi;
}
snprintf(fc_host_serial_number(lport->host),
@@ -868,6 +868,7 @@ static void fcoe_fdmi_info(struct fc_lport *lport, struct net_device *netdev)
/* Enable FDMI lport states */
lport->fdmi_enabled = 1;
+free_fdmi:
kfree(fdmi);
} else {
lport->fdmi_enabled = 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2] scsi: fcoe: add missed kfree() in an error path
2020-07-09 12:05 [PATCH v2] scsi: fcoe: add missed kfree() in an error path Jing Xiangfeng
@ 2020-07-13 17:53 ` Ewan D. Milne
2020-07-16 8:13 ` Jing Xiangfeng
0 siblings, 1 reply; 3+ messages in thread
From: Ewan D. Milne @ 2020-07-13 17:53 UTC (permalink / raw)
To: Jing Xiangfeng, hare, jejb, martin.petersen, robert.w.love,
Neerav.Parikh, Markus.Elfring
Cc: linux-scsi, linux-kernel
See below.
On Thu, 2020-07-09 at 20:05 +0800, Jing Xiangfeng wrote:
> fcoe_fdmi_info() misses to call kfree() in an error path.
> Add a label 'free_fdmi' and jump to it.
>
> Fixes: f07d46bbc9ba ("fcoe: Fix smatch warning in fcoe_fdmi_info
> function")
> Signed-off-by: Jing Xiangfeng <jingxiangfeng@huawei.com>
> ---
> drivers/scsi/fcoe/fcoe.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index 25dae9f0b205..a63057a03772 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -830,7 +830,7 @@ static void fcoe_fdmi_info(struct fc_lport
> *lport, struct net_device *netdev)
> if (rc) {
> printk(KERN_INFO "fcoe: Failed to retrieve FDMI
> "
> "information from netdev.\n");
> - return;
> + goto free_fdmi;
> }
>
> snprintf(fc_host_serial_number(lport->host),
> @@ -868,6 +868,7 @@ static void fcoe_fdmi_info(struct fc_lport
> *lport, struct net_device *netdev)
>
> /* Enable FDMI lport states */
> lport->fdmi_enabled = 1;
> +free_fdmi:
> kfree(fdmi);
> } else {
> lport->fdmi_enabled = 0;
Normally I would like to see goto labels for error paths outside
conditionals and at the end of the function. In this case it would
seem to be cleaner to put an else { } clause in the if (rc) above
around the snprintf() calls.
-Ewan
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] scsi: fcoe: add missed kfree() in an error path
2020-07-13 17:53 ` Ewan D. Milne
@ 2020-07-16 8:13 ` Jing Xiangfeng
0 siblings, 0 replies; 3+ messages in thread
From: Jing Xiangfeng @ 2020-07-16 8:13 UTC (permalink / raw)
To: Ewan D. Milne, hare, jejb, martin.petersen, robert.w.love,
Neerav.Parikh, Markus.Elfring
Cc: linux-scsi, linux-kernel
On 2020/7/14 1:53, Ewan D. Milne wrote:
> See below.
>
> On Thu, 2020-07-09 at 20:05 +0800, Jing Xiangfeng wrote:
>> fcoe_fdmi_info() misses to call kfree() in an error path.
>> Add a label 'free_fdmi' and jump to it.
>>
>> Fixes: f07d46bbc9ba ("fcoe: Fix smatch warning in fcoe_fdmi_info
>> function")
>> Signed-off-by: Jing Xiangfeng <jingxiangfeng@huawei.com>
>> ---
>> drivers/scsi/fcoe/fcoe.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
>> index 25dae9f0b205..a63057a03772 100644
>> --- a/drivers/scsi/fcoe/fcoe.c
>> +++ b/drivers/scsi/fcoe/fcoe.c
>> @@ -830,7 +830,7 @@ static void fcoe_fdmi_info(struct fc_lport
>> *lport, struct net_device *netdev)
>> if (rc) {
>> printk(KERN_INFO "fcoe: Failed to retrieve FDMI
>> "
>> "information from netdev.\n");
>> - return;
>> + goto free_fdmi;
>> }
>>
>> snprintf(fc_host_serial_number(lport->host),
>> @@ -868,6 +868,7 @@ static void fcoe_fdmi_info(struct fc_lport
>> *lport, struct net_device *netdev)
>>
>> /* Enable FDMI lport states */
>> lport->fdmi_enabled = 1;
>> +free_fdmi:
>> kfree(fdmi);
>> } else {
>> lport->fdmi_enabled = 0;
>
> Normally I would like to see goto labels for error paths outside
> conditionals and at the end of the function.
I agree.
In this case it would
> seem to be cleaner to put an else { } clause in the if (rc) above
> around the snprintf() calls.
It is ok with me. v1 is also a simpler way.
+Hannes, Which is preferable?
Thanks
>
> -Ewan
>
> .
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-07-16 8:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-09 12:05 [PATCH v2] scsi: fcoe: add missed kfree() in an error path Jing Xiangfeng
2020-07-13 17:53 ` Ewan D. Milne
2020-07-16 8:13 ` Jing Xiangfeng
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.