Re: [PATCH net-next] cxgb4: remove redundant NULL check

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:

quoted

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 <redacted>
Signed-off-by: Yang Li <redacted>

quoted

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.