svn commit: r263113 - head/sys/x86/x86

Bruce Evans brde at optusnet.com.au
Thu Mar 13 21:07:38 UTC 2014


On Thu, 13 Mar 2014, John Baldwin wrote:

> Log:
>  Correct type for malloc().
>
>  Submitted by:	"Conrad Meyer" <conrad.meyer at isilon.com>

Not so nice a cleanup as a previous one by the same author.

> Modified: head/sys/x86/x86/mca.c
> ==============================================================================
> --- head/sys/x86/x86/mca.c	Thu Mar 13 16:51:40 2014	(r263112)
> +++ head/sys/x86/x86/mca.c	Thu Mar 13 18:11:42 2014	(r263113)
> @@ -700,8 +700,8 @@ cmci_setup(void)
> {
> 	int i;
>
> -	cmc_state = malloc((mp_maxid + 1) * sizeof(struct cmc_state **),
> -	    M_MCA, M_WAITOK);
> +	cmc_state = malloc((mp_maxid + 1) * sizeof(struct cmc_state *), M_MCA,
> +	    M_WAITOK);

This still spells the element type verbosely and non-robustly as
<element type> instead of as <element object>.  Better spelling:

 	cmc_state = malloc((mp_maxid + 1) * sizeof(*cmc_state), M_MCA,
 	    M_WAITOK);

The variable names are confusing.  cmc_state is a doubly-indirect pointer,
but is spelled without a single p.  This was confusing enough to give the
original bug (but since all struct pointers have the same size even in
strictly portable C, this was only a style bug).  It is unclear what sort
of a pointer cmc_state is in the changed version, but using the correct
spelling automatically gets the number of stars right (since the size being
malloc()ed has a product term with a variable, the allocation must be for a
dynamic array and the element type must need a single star to modify the
result type; this star actually removes 1 of the 2 indirections, giving
a pointer to the basic struct type).

Local pointers to struct cmc_state are mostly spelled cc.

Struct member names in struct cmc_state are missing prefixes.

> 	for (i = 0; i <= mp_maxid; i++)
> 		cmc_state[i] = malloc(sizeof(struct cmc_state) * mca_banks,
> 		    M_MCA, M_WAITOK | M_ZERO);

SImilarly, plus the order of terms in the multiplication is gratuitously
different.  I prefer the first order:

 		cmc_state[i] = malloc(mca_banks * sizeof(*cmc_state[i]),
 		    M_MCA, M_WAITOK | M_ZERO);

There are 2 other malloc()s in the file.  These use the best spelling
for the sizeof() but not for variable names.

Bruce


More information about the svn-src-head mailing list