| Message ID | 1611629413-81373-1-git-send-email-abaci-bugfix@linux.alibaba.com |
|---|---|
| State | New |
| Headers | show |
| Series | [net-next] cxgb4: remove redundant NULL check | expand |
On Tuesday, January 01/26/21, 2021 at 10:50:13 +0800, Yang Li wrote: > Fix below warnings reported by coccicheck: > ./drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c:323:3-9: WARNING: > NULL check before some freeing functions is not needed. > ./drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c:3554:2-8: WARNING: > NULL check before some freeing functions is not needed. > ./drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c:157:2-7: WARNING: > NULL check before some freeing functions is not needed. > ./drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c:525:3-9: WARNING: > NULL check before some freeing functions is not needed. > > Reported-by: Abaci Robot <abaci@linux.alibaba.com> > Signed-off-by: Yang Li <abaci-bugfix@linux.alibaba.com> > --- > drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c | 3 +-- > drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c | 3 +-- > drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c | 3 +-- > drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c | 6 ++---- > 4 files changed, 5 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c b/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c > index ce28820..12fcf84 100644 > --- a/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c > +++ b/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c > @@ -323,8 +323,7 @@ void t4_cleanup_clip_tbl(struct adapter *adap) > struct clip_tbl *ctbl = adap->clipt; > > if (ctbl) { > - if (ctbl->cl_list) > - kvfree(ctbl->cl_list); > + kvfree(ctbl->cl_list); > kvfree(ctbl); > } > } > diff --git a/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c b/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c > index 75474f8..94eb8a6 100644 > --- a/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c > +++ b/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c > @@ -3554,8 +3554,7 @@ int cudbg_collect_qdesc(struct cudbg_init *pdbg_init, > } > > out_free: > - if (data) > - kvfree(data); > + kvfree(data); > > #undef QDESC_GET_FLQ > #undef QDESC_GET_RXQ > diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c > index 77648e4..dd66b24 100644 > --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c > +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c > @@ -157,8 +157,7 @@ static int cudbg_alloc_compress_buff(struct cudbg_init *pdbg_init) > > static void cudbg_free_compress_buff(struct cudbg_init *pdbg_init) > { > - if (pdbg_init->compress_buff) NAK. The above check is necessary. pdbg_init->compress_buff may be NULL when Zlib is unavailable or when pdbg_init->compress_buff allocation fails, in which case we ignore error and continue without compression. Check is necessary before calling vfree(). > - vfree(pdbg_init->compress_buff); > + vfree(pdbg_init->compress_buff); > } > > int cxgb4_cudbg_collect(struct adapter *adap, void *buf, u32 *buf_size, > diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c > index dede025..97a811f 100644 > --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c > +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c > @@ -525,12 +525,10 @@ struct cxgb4_tc_u32_table *cxgb4_init_tc_u32(struct adapter *adap) > for (i = 0; i < t->size; i++) { > struct cxgb4_link *link = &t->table[i]; > > - if (link->tid_map) > - kvfree(link->tid_map); > + kvfree(link->tid_map); The above change is wrong. NAK. If the call to link->tid_map = kvcalloc() above fails, it still goes ahead and calls kvfree(link->tid_map) even for failed cases, which is wrong. Check is necessary before calling kvfree(). > } > > - if (t) > - kvfree(t); > + kvfree(t); > > return NULL; > } > -- > 1.8.3.1 >
On Wed, 27 Jan 2021 12:04:27 +0530 Raju Rangoju wrote: > On Tuesday, January 01/26/21, 2021 at 10:50:13 +0800, Yang Li wrote: > > Fix below warnings reported by coccicheck: > > ./drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c:323:3-9: WARNING: > > NULL check before some freeing functions is not needed. > > ./drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c:3554:2-8: WARNING: > > NULL check before some freeing functions is not needed. > > ./drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c:157:2-7: WARNING: > > NULL check before some freeing functions is not needed. > > ./drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c:525:3-9: WARNING: > > NULL check before some freeing functions is not needed. > > > > Reported-by: Abaci Robot <abaci@linux.alibaba.com> > > Signed-off-by: Yang Li <abaci-bugfix@linux.alibaba.com> > > diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c > > index 77648e4..dd66b24 100644 > > --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c > > +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c > > @@ -157,8 +157,7 @@ static int cudbg_alloc_compress_buff(struct cudbg_init *pdbg_init) > > > > static void cudbg_free_compress_buff(struct cudbg_init *pdbg_init) > > { > > - if (pdbg_init->compress_buff) > > NAK. The above check is necessary. > > pdbg_init->compress_buff may be NULL when Zlib is unavailable or when > pdbg_init->compress_buff allocation fails, in which case we ignore error > and continue without compression. Check is necessary before calling > vfree(). Thanks taking a look! The point is that vfree() kfree() etc. all can be fed NULL in which case they are a nop. E.g.: /** * vfree - Release memory allocated by vmalloc() * @addr: Memory base address * * Free the virtually continuous memory area starting at @addr, as obtained * from one of the vmalloc() family of APIs. This will usually also free the * physical memory underlying the virtual allocation, but that memory is * reference counted, so it will not be freed until the last user goes away. * vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv >* If @addr is NULL, no operation is performed. <= ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ * * Context: * May sleep if called *not* from interrupt context. * Must not be called in NMI context (strictly speaking, it could be * if we have CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG, but making the calling * conventions for vfree() arch-depenedent would be a really bad idea). */ I don't think there is any advanced static analysis going on here if that's what you assumed. Yang, please respin, and explain in the patch message why removing those conditions is safe.
diff --git a/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c b/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c index ce28820..12fcf84 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c +++ b/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c @@ -323,8 +323,7 @@ void t4_cleanup_clip_tbl(struct adapter *adap) struct clip_tbl *ctbl = adap->clipt; if (ctbl) { - if (ctbl->cl_list) - kvfree(ctbl->cl_list); + kvfree(ctbl->cl_list); kvfree(ctbl); } } diff --git a/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c b/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c index 75474f8..94eb8a6 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c +++ b/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c @@ -3554,8 +3554,7 @@ int cudbg_collect_qdesc(struct cudbg_init *pdbg_init, } out_free: - if (data) - kvfree(data); + kvfree(data); #undef QDESC_GET_FLQ #undef QDESC_GET_RXQ diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c index 77648e4..dd66b24 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c @@ -157,8 +157,7 @@ static int cudbg_alloc_compress_buff(struct cudbg_init *pdbg_init) static void cudbg_free_compress_buff(struct cudbg_init *pdbg_init) { - if (pdbg_init->compress_buff) - vfree(pdbg_init->compress_buff); + vfree(pdbg_init->compress_buff); } int cxgb4_cudbg_collect(struct adapter *adap, void *buf, u32 *buf_size, diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c index dede025..97a811f 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c @@ -525,12 +525,10 @@ struct cxgb4_tc_u32_table *cxgb4_init_tc_u32(struct adapter *adap) for (i = 0; i < t->size; i++) { struct cxgb4_link *link = &t->table[i]; - if (link->tid_map) - kvfree(link->tid_map); + kvfree(link->tid_map); } - if (t) - kvfree(t); + kvfree(t); return NULL; }
Fix below warnings reported by coccicheck: ./drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c:323:3-9: WARNING: NULL check before some freeing functions is not needed. ./drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c:3554:2-8: WARNING: NULL check before some freeing functions is not needed. ./drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c:157:2-7: WARNING: NULL check before some freeing functions is not needed. ./drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c:525:3-9: WARNING: NULL check before some freeing functions is not needed. Reported-by: Abaci Robot <abaci@linux.alibaba.com> Signed-off-by: Yang Li <abaci-bugfix@linux.alibaba.com> --- drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c | 3 +-- drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c | 3 +-- drivers/net/ethernet/chelsio/cxgb4/cxgb4_cudbg.c | 3 +-- drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c | 6 ++---- 4 files changed, 5 insertions(+), 10 deletions(-)