Re: [PATCH v1] lpfc 8.3.37: Remove redundant NULL check before kfree
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-ow...@vger.kernel.org [mailto:linux-scsi- ow...@vger.kernel.org] On Behalf Of James Smart Sent: Wednesday, 06 March, 2013 3:10 PM To: syamsidha...@gmail.com Cc: linux-scsi@vger.kernel.org; jbottom...@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, syamsidha...@gmail.com wrote: From: Syam Sidhardhan s.s...@samsung.com kfree on NULL pointer is a no-op. Signed-off-by: Syam Sidhardhan s.s...@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 majord...@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 majord...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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, syamsidha...@gmail.com wrote: From: Syam Sidhardhan s.s...@samsung.com kfree on NULL pointer is a no-op. Signed-off-by: Syam Sidhardhan s.s...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1] lpfc 8.3.37: Remove redundant NULL check before kfree
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-ow...@vger.kernel.org [mailto:linux-scsi- ow...@vger.kernel.org] On Behalf Of James Smart Sent: Wednesday, 06 March, 2013 3:10 PM To: syamsidha...@gmail.com Cc: linux-scsi@vger.kernel.org; jbottom...@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, syamsidha...@gmail.com wrote: From: Syam Sidhardhan s.s...@samsung.com kfree on NULL pointer is a no-op. Signed-off-by: Syam Sidhardhan s.s...@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 majord...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
