* [PATCH v1] lpfc 8.3.37: Remove redundant NULL check before kfree
@ 2013-03-06 20:12 syamsidhardh
2013-03-06 21:10 ` James Smart
0 siblings, 1 reply; 4+ messages in thread
From: syamsidhardh @ 2013-03-06 20:12 UTC (permalink / raw)
To: linux-scsi; +Cc: syamsidhardh, JBottomley, james.smart, Syam Sidhardhan
From: Syam Sidhardhan <s.syam@samsung.com>
kfree on NULL pointer is a no-op.
Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
---
v1-> Corrected the from address.
drivers/scsi/lpfc/lpfc_bsg.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_bsg.c b/drivers/scsi/lpfc/lpfc_bsg.c
index 32d5683..2166097 100644
--- a/drivers/scsi/lpfc/lpfc_bsg.c
+++ b/drivers/scsi/lpfc/lpfc_bsg.c
@@ -1129,8 +1129,7 @@ lpfc_bsg_hba_set_event(struct fc_bsg_job *job)
return 0; /* call job done later */
job_error:
- if (dd_data != NULL)
- kfree(dd_data);
+ kfree(dd_data);
job->dd_data = NULL;
return rc;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v1] lpfc 8.3.37: Remove redundant NULL check before kfree
2013-03-06 20:12 [PATCH v1] lpfc 8.3.37: Remove redundant NULL check before kfree syamsidhardh
@ 2013-03-06 21:10 ` James Smart
2013-03-06 23:32 ` Elliott, Robert (Server Storage)
0 siblings, 1 reply; 4+ messages in thread
From: James Smart @ 2013-03-06 21:10 UTC (permalink / raw)
To: syamsidhardh; +Cc: linux-scsi, JBottomley, Syam Sidhardhan, Smart, James
Syam,
Thank you for the patch - it is valid.
However, I prefer not to merge this. I would rather force the coder to
think about the pointer value explicitly rather than depending on the
convenience/one line optimization. We've had errors in the past covered
up by this gracious behavior. Additionally, we have coders that work on
linux and vmware, and the semantics of the kfree() routine differ. For
now, I'd prefer to stay as is and force good habits.
-- james s
On 3/6/2013 3:12 PM, syamsidhardh@gmail.com wrote:
> From: Syam Sidhardhan <s.syam@samsung.com>
>
> kfree on NULL pointer is a no-op.
>
> Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
> ---
> v1-> Corrected the from address.
>
> drivers/scsi/lpfc/lpfc_bsg.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/lpfc/lpfc_bsg.c b/drivers/scsi/lpfc/lpfc_bsg.c
> index 32d5683..2166097 100644
> --- a/drivers/scsi/lpfc/lpfc_bsg.c
> +++ b/drivers/scsi/lpfc/lpfc_bsg.c
> @@ -1129,8 +1129,7 @@ lpfc_bsg_hba_set_event(struct fc_bsg_job *job)
> return 0; /* call job done later */
>
> job_error:
> - if (dd_data != NULL)
> - kfree(dd_data);
> + kfree(dd_data);
>
> job->dd_data = NULL;
> return rc;
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v1] lpfc 8.3.37: Remove redundant NULL check before kfree
2013-03-06 21:10 ` James Smart
@ 2013-03-06 23:32 ` Elliott, Robert (Server Storage)
2013-03-07 18:05 ` James Smart
0 siblings, 1 reply; 4+ messages in thread
From: Elliott, Robert (Server Storage) @ 2013-03-06 23:32 UTC (permalink / raw)
To: James.Smart, syamsidhardh; +Cc: linux-scsi, JBottomley, Syam Sidhardhan
If the other approach is taken, then not all kfree() calls are protected by a NULL check.
One example in lpfc_els.c (from 3.7-rc5):
if (!pbuflist || !pbuflist->virt)
goto els_iocb_free_pbuf_exit;
...
els_iocb_free_pbuf_exit:
if (expectRsp)
lpfc_mbuf_free(phba, prsp->virt, prsp->phys);
kfree(pbuflist);
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of James Smart
> Sent: Wednesday, 06 March, 2013 3:10 PM
> To: syamsidhardh@gmail.com
> Cc: linux-scsi@vger.kernel.org; JBottomley@parallels.com; Syam Sidhardhan;
> Smart, James
> Subject: Re: [PATCH v1] lpfc 8.3.37: Remove redundant NULL check before
> kfree
>
> Syam,
>
> Thank you for the patch - it is valid.
>
> However, I prefer not to merge this. I would rather force the coder to
> think about the pointer value explicitly rather than depending on the
> convenience/one line optimization. We've had errors in the past covered
> up by this gracious behavior. Additionally, we have coders that work on
> linux and vmware, and the semantics of the kfree() routine differ. For
> now, I'd prefer to stay as is and force good habits.
>
> -- james s
>
>
> On 3/6/2013 3:12 PM, syamsidhardh@gmail.com wrote:
> > From: Syam Sidhardhan <s.syam@samsung.com>
> >
> > kfree on NULL pointer is a no-op.
> >
> > Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
> > ---
> > v1-> Corrected the from address.
> >
> > drivers/scsi/lpfc/lpfc_bsg.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/scsi/lpfc/lpfc_bsg.c b/drivers/scsi/lpfc/lpfc_bsg.c
> > index 32d5683..2166097 100644
> > --- a/drivers/scsi/lpfc/lpfc_bsg.c
> > +++ b/drivers/scsi/lpfc/lpfc_bsg.c
> > @@ -1129,8 +1129,7 @@ lpfc_bsg_hba_set_event(struct fc_bsg_job *job)
> > return 0; /* call job done later */
> >
> > job_error:
> > - if (dd_data != NULL)
> > - kfree(dd_data);
> > + kfree(dd_data);
> >
> > job->dd_data = NULL;
> > return rc;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] lpfc 8.3.37: Remove redundant NULL check before kfree
2013-03-06 23:32 ` Elliott, Robert (Server Storage)
@ 2013-03-07 18:05 ` James Smart
0 siblings, 0 replies; 4+ messages in thread
From: James Smart @ 2013-03-07 18:05 UTC (permalink / raw)
To: Elliott, Robert (Server Storage)
Cc: syamsidhardh, linux-scsi, JBottomley, Syam Sidhardhan
I don't disagree. My intent would be it is all one way - with my
leaning toward being explicit. Unfortunately, it's a low priority task.
-- james s
On 3/6/2013 6:32 PM, Elliott, Robert (Server Storage) wrote:
> If the other approach is taken, then not all kfree() calls are protected by a NULL check.
>
> One example in lpfc_els.c (from 3.7-rc5):
> if (!pbuflist || !pbuflist->virt)
> goto els_iocb_free_pbuf_exit;
> ...
> els_iocb_free_pbuf_exit:
> if (expectRsp)
> lpfc_mbuf_free(phba, prsp->virt, prsp->phys);
> kfree(pbuflist);
>
>
>
>
>> -----Original Message-----
>> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
>> owner@vger.kernel.org] On Behalf Of James Smart
>> Sent: Wednesday, 06 March, 2013 3:10 PM
>> To: syamsidhardh@gmail.com
>> Cc: linux-scsi@vger.kernel.org; JBottomley@parallels.com; Syam Sidhardhan;
>> Smart, James
>> Subject: Re: [PATCH v1] lpfc 8.3.37: Remove redundant NULL check before
>> kfree
>>
>> Syam,
>>
>> Thank you for the patch - it is valid.
>>
>> However, I prefer not to merge this. I would rather force the coder to
>> think about the pointer value explicitly rather than depending on the
>> convenience/one line optimization. We've had errors in the past covered
>> up by this gracious behavior. Additionally, we have coders that work on
>> linux and vmware, and the semantics of the kfree() routine differ. For
>> now, I'd prefer to stay as is and force good habits.
>>
>> -- james s
>>
>>
>> On 3/6/2013 3:12 PM, syamsidhardh@gmail.com wrote:
>>> From: Syam Sidhardhan <s.syam@samsung.com>
>>>
>>> kfree on NULL pointer is a no-op.
>>>
>>> Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
>>> ---
>>> v1-> Corrected the from address.
>>>
>>> drivers/scsi/lpfc/lpfc_bsg.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/scsi/lpfc/lpfc_bsg.c b/drivers/scsi/lpfc/lpfc_bsg.c
>>> index 32d5683..2166097 100644
>>> --- a/drivers/scsi/lpfc/lpfc_bsg.c
>>> +++ b/drivers/scsi/lpfc/lpfc_bsg.c
>>> @@ -1129,8 +1129,7 @@ lpfc_bsg_hba_set_event(struct fc_bsg_job *job)
>>> return 0; /* call job done later */
>>>
>>> job_error:
>>> - if (dd_data != NULL)
>>> - kfree(dd_data);
>>> + kfree(dd_data);
>>>
>>> job->dd_data = NULL;
>>> return rc;
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-03-07 18:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-06 20:12 [PATCH v1] lpfc 8.3.37: Remove redundant NULL check before kfree syamsidhardh
2013-03-06 21:10 ` James Smart
2013-03-06 23:32 ` Elliott, Robert (Server Storage)
2013-03-07 18:05 ` James Smart
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.