Re: [v3 01/11] ASoC: Intel: mrfld - add the dsp sst driver

From: Mark Brown <broonie@kernel.org>
To: "Subhransu S. Prusty" <subhransu.s.prusty@intel.com>
Cc: vinod.koul@intel.com, alsa-devel@alsa-project.org,
	Lars-Peter Clausen <lars@metafoo.de>,
	lgirdwood@gmail.com
Subject: Re: [v3 01/11] ASoC: Intel: mrfld - add the dsp sst driver
Date: Mon, 1 Sep 2014 12:15:58 +0100	[thread overview]
Message-ID: <20140901111557.GR29327@sirena.org.uk> (raw)
In-Reply-To: <20140901103751.GA12898@vinod.koul@linux.intel.com>

[-- Attachment #1.1: Type: text/plain, Size: 1053 bytes --]

On Mon, Sep 01, 2014 at 04:07:55PM +0530, Subhransu S. Prusty wrote:
> On Wed, Aug 27, 2014 at 08:40:21PM +0100, Mark Brown wrote:
> > On Thu, Aug 21, 2014 at 06:20:40PM +0530, Subhransu S. Prusty wrote:

> > > +static inline unsigned int sst_assign_pvt_id(struct intel_sst_drv *sst_drv_ctx)
> > > +{
> > > +	unsigned int local;
> > > +
> > > +	spin_lock(&sst_drv_ctx->block_lock);
> > > +	sst_drv_ctx->pvt_id++;
> > > +	if (sst_drv_ctx->pvt_id > MAX_BLOCKS)
> > > +		sst_drv_ctx->pvt_id = 1;
> > > +	local = sst_drv_ctx->pvt_id;
> > > +	spin_unlock(&sst_drv_ctx->block_lock);
> > > +	return local;
> > > +}

> > The comments about overflow continue to apply here.

> Vinod has already replied to you regarding this, below is a snippet from
> the conversation earlier. Is there anything else we need to address?

Yes, and as I said I just don't buy that argument.  It's code that has
fairly clear problems on inspection, saying "we've not hit the problem
yet" isn't particularly persuasive.  Something like a new firmware could
easily trigger problems.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]