From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Carpenter Date: Wed, 24 Mar 2010 14:06:21 +0000 Subject: Re: [rfc patch] wm8994: range checking issue Message-Id: <20100324140621.GI21571@bicker> List-Id: References: <20100324120107.GH21571@bicker> <20100324125946.GA26453@rakim.wolfsonmicro.main> In-Reply-To: <20100324125946.GA26453@rakim.wolfsonmicro.main> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Mark Brown Cc: Liam Girdwood , Jaroslav Kysela , Takashi Iwai , Joonyoung Shim , alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org On Wed, Mar 24, 2010 at 12:59:46PM +0000, Mark Brown wrote: > On Wed, Mar 24, 2010 at 03:01:07PM +0300, Dan Carpenter wrote: > > Smatch complained about BUG_ON(reg > WM8994_MAX_REGISTER) because the > > actual number of elements in the array was WM8994_REG_CACHE_SIZE + 1. > > > I changed the BUG_ON() to return -EINVAL. > > Please don't introduce orthogonal changes like this in patches, it's bad > practice and increases the chances of your patch being nacked. > > > I was confused why WM8994_REG_CACHE_SIZE was different from the actual > > size of ->reg_cache and I was concerned because some places used > > ARRAY_SIZE() to find the end of the array and other places used > > WM8994_REG_CACHE_SIZE. In my patch, I made them the same. > > This is caused by confusion with the MAX_CACHED_REGISTER definition in > the header. Best to use that one consistently, I guess - I've got a > sneaking suspicion something has gone AWOL in the driver publication > process. Hm... That sounds more involved than I anticipated. I don't have the hardware and don't feel comfortable making complicated changes if I can't test them. Can someone else take care of this. regards, dan carpenter