| Message ID | 20190307231534.2640-1-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] scsi: qla4xxx: Don't call dma_free_coherent when buf is NULL | expand |
On Thu, Mar 7, 2019 at 3:17 PM Nathan Chancellor <natechancellor@gmail.com> wrote: > > When building with -Wsometimes-uninitialized, Clang warns: > > drivers/scsi/qla4xxx/ql4_os.c:5915:7: warning: variable 'buf_dma' is > used uninitialized whenever 'if' condition is false > [-Wsometimes-uninitialized] > > Don't call dma_free_coherent when buf is NULL, meaning that we never > called dma_alloc_coherent and initialized buf_dma. > > Fixes: 2a991c215978 ("[SCSI] qla4xxx: Boot from SAN support for open-iscsi") > Link: https://github.com/ClangBuiltLinux/linux/issues/391 > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > > v1 -> v2: > > * Check that buf is not NULL, which matches how dma_free_coherent is > called in the rest of the tree. This still fixes the warning. > > drivers/scsi/qla4xxx/ql4_os.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c > index 16a18d5d856f..9f56dafc3cda 100644 > --- a/drivers/scsi/qla4xxx/ql4_os.c > +++ b/drivers/scsi/qla4xxx/ql4_os.c > @@ -5982,7 +5982,8 @@ static int get_fw_boot_info(struct scsi_qla_host *ha, uint16_t ddb_index[]) > ddb_index[1])); > > exit_boot_info_free: > - dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma); > + if (buf) > + dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma); Same thoughts as: https://lkml.org/lkml/2019/3/8/786 WDYT?
On Fri, Mar 8, 2019 at 10:17 PM 'Nick Desaulniers' via Clang Built Linux <clang-built-linux@googlegroups.com> wrote: > > On Thu, Mar 7, 2019 at 3:17 PM Nathan Chancellor > <natechancellor@gmail.com> wrote: > > > > When building with -Wsometimes-uninitialized, Clang warns: > > > > drivers/scsi/qla4xxx/ql4_os.c:5915:7: warning: variable 'buf_dma' is > > used uninitialized whenever 'if' condition is false > > [-Wsometimes-uninitialized] > > > > Don't call dma_free_coherent when buf is NULL, meaning that we never > > called dma_alloc_coherent and initialized buf_dma. > > > > Fixes: 2a991c215978 ("[SCSI] qla4xxx: Boot from SAN support for open-iscsi") > > Link: https://github.com/ClangBuiltLinux/linux/issues/391 > > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > > diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c > > index 16a18d5d856f..9f56dafc3cda 100644 > > --- a/drivers/scsi/qla4xxx/ql4_os.c > > +++ b/drivers/scsi/qla4xxx/ql4_os.c > > @@ -5982,7 +5982,8 @@ static int get_fw_boot_info(struct scsi_qla_host *ha, uint16_t ddb_index[]) > > ddb_index[1])); > > > > exit_boot_info_free: > > - dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma); > > + if (buf) > > + dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma); > > Same thoughts as: https://lkml.org/lkml/2019/3/8/786 > WDYT? Initializing the variable is definitely worse here, don't listen to clang's bad suggestions ;-) This was my fix, I'll send that as a proper patch as well: --- a/drivers/scsi/qla4xxx/ql4_os.c +++ b/drivers/scsi/qla4xxx/ql4_os.c @@ -5928,7 +5928,7 @@ static int get_fw_boot_info(struct scsi_qla_host *ha, uint16_t ddb_index[]) val = rd_nvram_byte(ha, sec_addr); if (val & BIT_7) ddb_index[1] = (val & 0x7f); - + goto exit_boot_info; } else if (is_qla80XX(ha)) { buf = dma_alloc_coherent(&ha->pdev->dev, size, &buf_dma, GFP_KERNEL); Arnd
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c index 16a18d5d856f..9f56dafc3cda 100644 --- a/drivers/scsi/qla4xxx/ql4_os.c +++ b/drivers/scsi/qla4xxx/ql4_os.c @@ -5982,7 +5982,8 @@ static int get_fw_boot_info(struct scsi_qla_host *ha, uint16_t ddb_index[]) ddb_index[1])); exit_boot_info_free: - dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma); + if (buf) + dma_free_coherent(&ha->pdev->dev, size, buf, buf_dma); exit_boot_info: ha->pri_ddb_idx = ddb_index[0]; ha->sec_ddb_idx = ddb_index[1];
When building with -Wsometimes-uninitialized, Clang warns: drivers/scsi/qla4xxx/ql4_os.c:5915:7: warning: variable 'buf_dma' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] Don't call dma_free_coherent when buf is NULL, meaning that we never called dma_alloc_coherent and initialized buf_dma. Fixes: 2a991c215978 ("[SCSI] qla4xxx: Boot from SAN support for open-iscsi") Link: https://github.com/ClangBuiltLinux/linux/issues/391 Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- v1 -> v2: * Check that buf is not NULL, which matches how dma_free_coherent is called in the rest of the tree. This still fixes the warning. drivers/scsi/qla4xxx/ql4_os.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)