[patch] Staging: intel_sst: off by one bug

[patch] Staging: intel_sst: off by one bug

[Dan Carpenter]

This should be >= instead of > or we go passed the end of the array.

Also the arrays are declared with size MAX_NUM_STREAMS.  This is the
only place that uses MAX_NUM_STREAMS_MFLD.  It seems like asking for
trouble to use two variables for the same information.  I've changed
everything to use MAX_NUM_STREAMS.

This bug isn't really harmful.  In the worst case, if you enabled
debugging then you would see a message.

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/drivers/staging/intel_sst/intel_sst_fw_ipc.h b/drivers/staging/intel_sst/intel_sst_fw_ipc.h
index 1a2f67f..9d3c368 100644
--- a/drivers/staging/intel_sst/intel_sst_fw_ipc.h
+++ b/drivers/staging/intel_sst/intel_sst_fw_ipc.h
@@ -31,7 +31,6 @@
 */
 
 #define MAX_NUM_STREAMS_MRST 3
-#define MAX_NUM_STREAMS_MFLD 6
 #define MAX_NUM_STREAMS 6
 #define MAX_DBG_RW_BYTES 80
 #define MAX_NUM_SCATTER_BUFFERS 8
diff --git a/drivers/staging/intel_sst/intel_sst_stream.c b/drivers/staging/intel_sst/intel_sst_stream.c
index 1ce3a9c..b2c4b70 100644
--- a/drivers/staging/intel_sst/intel_sst_stream.c
+++ b/drivers/staging/intel_sst/intel_sst_stream.c
@@ -45,7 +45,7 @@
  */
 int sst_check_device_type(u32 device, u32 num_chan, u32 *pcm_slot)
 {
-	if (device > MAX_NUM_STREAMS_MFLD) {
+	if (device >= MAX_NUM_STREAMS) {
 		pr_debug("sst: device type invalid %d\n", device);
 		return -EINVAL;
 	}

Re: [patch] Staging: intel_sst: off by one bug

[Dan Carpenter]

Btw I don't understand the sst_drv_ctx->streams[] array.

What is stored in streams[0]?  

What does MRST from MAX_NUM_STREAMS_MRST and get_mrst_stream_id() stand
for?  I understand that MRST streams are SND_SST_DEVICE_HEADSET,
SND_SST_DEVICE_IHF, and SND_SST_DEVICE_VIBRA.  What makes them different
from SND_SST_DEVICE_HAPTIC and SND_SST_DEVICE_CAPTURE?

regards,
dan carpenter

RE: [patch] Staging: intel_sst: off by one bug

[Koul, Vinod]

 
> Btw I don't understand the sst_drv_ctx->streams[] array.
This is used to store the runtime information related to the stream
> 
> What is stored in streams[0]?
Nothing :) its unused
In this array, the index refers to stream id which starts from 1 to x.
So I instead of adding stream id as field and subtracting one to get index, 
left [0] as unused.... 
> 
> What does MRST from MAX_NUM_STREAMS_MRST and get_mrst_stream_id() stand
> for?  I understand that MRST streams are SND_SST_DEVICE_HEADSET,
> SND_SST_DEVICE_IHF, and SND_SST_DEVICE_VIBRA.  What makes them different
> from SND_SST_DEVICE_HAPTIC and SND_SST_DEVICE_CAPTURE?
The driver supports two platforms, Moorestown (MRST) and Medfield (MFLD)
The capabilities of each are different. In former we support 3 streams
and in latter 5.
The driver assigns the stream id, for mrst, just in first come first server 
basis. In mfld, based on type of device opened, for instance Vibra will always 
be stream 3 and capture 5.

HTH
Vinod

RE: [patch] Staging: intel_sst: off by one bug

[Koul, Vinod]

> This should be >= instead of > or we go passed the end of the array.
> 
> Also the arrays are declared with size MAX_NUM_STREAMS.  This is the
> only place that uses MAX_NUM_STREAMS_MFLD.  It seems like asking for
> trouble to use two variables for the same information.  I've changed
> everything to use MAX_NUM_STREAMS.
> 
> This bug isn't really harmful.  In the worst case, if you enabled
> debugging then you would see a message.
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Acked-by: Vinod Koul <vinod.koul@intel.com>

RE: [patch] Staging: intel_sst: off by one bug

[Harsha, Priya]

 
> This should be >= instead of > or we go passed the end of the array.
> 
> Also the arrays are declared with size MAX_NUM_STREAMS.  This is the
> only place that uses MAX_NUM_STREAMS_MFLD.  It seems like asking for
> trouble to use two variables for the same information.  I've changed
> everything to use MAX_NUM_STREAMS.
> 
> This bug isn't really harmful.  In the worst case, if you enabled
> debugging then you would see a message.
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
Acked-by: Harsha Priya <priya.harsha@intel.com>