From owner-svn-src-all@FreeBSD.ORG Thu Mar 13 21:07:38 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 666E120F; Thu, 13 Mar 2014 21:07:38 +0000 (UTC) Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by mx1.freebsd.org (Postfix) with ESMTP id 27F26EE1; Thu, 13 Mar 2014 21:07:37 +0000 (UTC) Received: from c122-106-147-133.carlnfd1.nsw.optusnet.com.au (c122-106-147-133.carlnfd1.nsw.optusnet.com.au [122.106.147.133]) by mail108.syd.optusnet.com.au (Postfix) with ESMTPS id 326CC1A07D5; Fri, 14 Mar 2014 08:07:30 +1100 (EST) Date: Fri, 14 Mar 2014 08:07:07 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: John Baldwin Subject: Re: svn commit: r263113 - head/sys/x86/x86 In-Reply-To: <201403131811.s2DIBgUb019577@svn.freebsd.org> Message-ID: <20140314073741.T978@besplex.bde.org> References: <201403131811.s2DIBgUb019577@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=ddC5gxne c=1 sm=1 tr=0 a=7NqvjVvQucbO2RlWB8PEog==:117 a=PO7r1zJSAAAA:8 a=ajY_dDsnAdgA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=5aufQkHnAAAA:8 a=nG4OoZy845CdLDuT8BwA:9 a=CjuIK1q_8ugA:10 a=WnMqR7Xl1tcA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Mar 2014 21:07:38 -0000 On Thu, 13 Mar 2014, John Baldwin wrote: > Log: > Correct type for malloc(). > > Submitted by: "Conrad Meyer" 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 instead of as . 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