Re: old bug in au88x0_core.c

[Takashi Iwai <tiwai@suse.de>]

On Mon, 27 Jun 2016 15:03:47 +0200,
Dan Carpenter wrote:
> 
> Hi ALSA devs,
> 
> The patch 1da177e4c3f4: "Linux-2.6.12-rc2" from Apr 16, 2005, leads
> to the following static checker warning:
> 
> 	sound/pci/au88x0/au88x0_core.c:1449 vortex_wtdma_bufshift()
> 	warn: mask and shift to zero
> 
> sound/pci/au88x0/au88x0_core.c
>   1441  static int vortex_wtdma_bufshift(vortex_t * vortex, int wtdma)
>   1442  {
>   1443          stream_t *dma = &vortex->dma_wt[wtdma];
>   1444          int page, p, pp, delta, i;
>   1445  
>   1446          page =
>   1447              (hwread(vortex->mmio, VORTEX_WTDMA_STAT + (wtdma << 2)) &
>   1448               WT_SUBBUF_MASK)
>   1449              >> WT_SUBBUF_SHIFT;
> 
> This is always zero because:
> 
> #define WT_SUBBUF_MASK 0x3
> #define WT_SUBBUF_SHIFT 0x15
> 
>   1450          if (dma->nr_periods >= 4)
>   1451                  delta = (page - dma->period_real) & 3;
>   1452          else {
>   1453                  delta = (page - dma->period_real);
>   1454                  if (delta < 0)
>   1455                          delta += dma->nr_periods;
>   1456          }
>   1457          if (delta == 0)
>   1458                  return 0;
> 
> 
>   1493  #if 0
>   1494  static void
>   1495  vortex_wtdma_getposition(vortex_t * vortex, int wtdma, int *subbuf, int *pos)
>   1496  {
>   1497          int temp;
>   1498          temp = hwread(vortex->mmio, VORTEX_WTDMA_STAT + (wtdma << 2));
>   1499          *subbuf = (temp >> WT_SUBBUF_SHIFT) & WT_SUBBUF_MASK;
> 
> >From this ifdef zero code, it looks like we should shift first then do
> the mask.
> 
>   1500          *pos = temp & POS_MASK;
>   1501  }
>   1502  
>   1503  static int vortex_wtdma_getcursubuffer(vortex_t * vortex, int wtdma)
>   1504  {
>   1505          return ((hwread(vortex->mmio, VORTEX_WTDMA_STAT + (wtdma << 2)) >>
>   1506                   POS_SHIFT) & POS_MASK);
>   1507  }
>   1508  #endif

Correct, this should be fixed in that way.  Below is the fix patch I'm
going to queue.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: au88x0: Fix calculation in vortex_wtdma_bufshift()

vortex_wtdma_bufshift() function does calculate the page index
wrongly, first masking then shift, which always results in zero.
The proper computation is to first shift, then mask.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/au88x0/au88x0_core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/sound/pci/au88x0/au88x0_core.c b/sound/pci/au88x0/au88x0_core.c
index 4a054d720112..d3125c169684 100644
--- a/sound/pci/au88x0/au88x0_core.c
+++ b/sound/pci/au88x0/au88x0_core.c
@@ -1444,9 +1444,8 @@ static int vortex_wtdma_bufshift(vortex_t * vortex, int wtdma)
 	int page, p, pp, delta, i;
 
 	page =
-	    (hwread(vortex->mmio, VORTEX_WTDMA_STAT + (wtdma << 2)) &
-	     WT_SUBBUF_MASK)
-	    >> WT_SUBBUF_SHIFT;
+	    (hwread(vortex->mmio, VORTEX_WTDMA_STAT + (wtdma << 2))
+	     >> WT_SUBBUF_SHIFT) & WT_SUBBUF_MASK;
 	if (dma->nr_periods >= 4)
 		delta = (page - dma->period_real) & 3;
 	else {
-- 
2.9.0