Re: [Outreachy kernel] [PATCH] staging: lustre: Modify arguments of sizeof() to pointer variables

* Re: [Outreachy kernel] [PATCH] staging: lustre: Modify arguments of sizeof() to pointer variables
  2016-02-10  7:32 [PATCH] staging: lustre: Modify arguments of sizeof() to pointer variables Janani Ravichandran
@ 2016-02-10  6:55 ` Julia Lawall
  2016-02-10  7:53   ` Janani Ravichandran
  0 siblings, 1 reply; 5+ messages in thread
From: Julia Lawall @ 2016-02-10  6:55 UTC (permalink / raw)
  To: Janani Ravichandran; +Cc: outreachy-kernel

On Wed, 10 Feb 2016, Janani Ravichandran wrote:

> It is better when sizeof() takes as an argument the size of the pointer variablewe care about rather than the type. This is because even if the type of the variable changes in the future, we will always get the correct size irrespective of the type.

You should respect the 80 character limit in the commit message.  This is 
a paragraph that is just a single line.

I'm not sure that the description has to be quite so detailed either.  It 
could be something like: Take the size of a dereference to a variable 
rather than the associated type, to make the code more resistent to type 
changes in the future.

Finally, did you do this with Coccinelle?  With Coccinelle, you can check 
that the type in the sizeof is actually the type of the variable.  The 
change is only correct in that case.

julia

> This patch makes such a change.
> 
> Signed-off-by: Janani Ravichandran <janani.rvchndrn@gmail.com>
> ---
>  drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> index cb74ae7..49cd333 100644
> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> @@ -1352,7 +1352,7 @@ static void kiblnd_destroy_fmr_pool(kib_fmr_pool_t *pool)
>  	if (pool->fpo_hdev != NULL)
>  		kiblnd_hdev_decref(pool->fpo_hdev);
>  
> -	LIBCFS_FREE(pool, sizeof(kib_fmr_pool_t));
> +	LIBCFS_FREE(pool, sizeof(*pool));
>  }
>  
>  static void kiblnd_destroy_fmr_pool_list(struct list_head *head)
> @@ -1410,7 +1410,7 @@ static int kiblnd_create_fmr_pool(kib_fmr_poolset_t *fps,
>  		CERROR("Failed to create FMR pool: %d\n", rc);
>  
>  		kiblnd_hdev_decref(fpo->fpo_hdev);
> -		LIBCFS_FREE(fpo, sizeof(kib_fmr_pool_t));
> +		LIBCFS_FREE(fpo, sizeof(*fpo));
>  		return rc;
>  	}
>  
> @@ -1458,7 +1458,7 @@ static int kiblnd_init_fmr_poolset(kib_fmr_poolset_t *fps, int cpt,
>  	kib_fmr_pool_t *fpo;
>  	int rc;
>  
> -	memset(fps, 0, sizeof(kib_fmr_poolset_t));
> +	memset(fps, 0, sizeof(*fps));
>  
>  	fps->fps_net = net;
>  	fps->fps_cpt = cpt;
> @@ -1606,7 +1606,7 @@ static void kiblnd_init_pool(kib_poolset_t *ps, kib_pool_t *pool, int size)
>  {
>  	CDEBUG(D_NET, "Initialize %s pool\n", ps->ps_name);
>  
> -	memset(pool, 0, sizeof(kib_pool_t));
> +	memset(pool, 0, sizeof(*pool));
>  	INIT_LIST_HEAD(&pool->po_free_list);
>  	pool->po_deadline = cfs_time_shift(IBLND_POOL_DEADLINE);
>  	pool->po_owner    = ps;
> @@ -1663,7 +1663,7 @@ static int kiblnd_init_poolset(kib_poolset_t *ps, int cpt,
>  	kib_pool_t *pool;
>  	int rc;
>  
> -	memset(ps, 0, sizeof(kib_poolset_t));
> +	memset(ps, 0, sizeof(*ps));
>  
>  	ps->ps_cpt          = cpt;
>  	ps->ps_net          = net;
> @@ -1834,7 +1834,7 @@ static void kiblnd_destroy_tx_pool(kib_pool_t *pool)
>  		    pool->po_size * sizeof(kib_tx_t));
>  out:
>  	kiblnd_fini_pool(pool);
> -	LIBCFS_FREE(tpo, sizeof(kib_tx_pool_t));
> +	LIBCFS_FREE(tpo, sizeof(*tpo));
>  }
>  
>  static int kiblnd_tx_pool_size(int ncpts)
> @@ -1866,7 +1866,7 @@ static int kiblnd_create_tx_pool(kib_poolset_t *ps, int size,
>  	npg = (size * IBLND_MSG_SIZE + PAGE_SIZE - 1) / PAGE_SIZE;
>  	if (kiblnd_alloc_pages(&tpo->tpo_tx_pages, ps->ps_cpt, npg) != 0) {
>  		CERROR("Can't allocate tx pages: %d\n", npg);
> -		LIBCFS_FREE(tpo, sizeof(kib_tx_pool_t));
> +		LIBCFS_FREE(tpo, sizeof(*tpo));
>  		return -ENOMEM;
>  	}
>  
> -- 
> 2.5.0
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160210073225.GA5118%40janani-Inspiron-3521.
> For more options, visit https://groups.google.com/d/optout.
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread