* Re: ALSA: lx6464es - driver for the digigram lx6464es interface
@ 2016-03-16 7:43 Dan Carpenter
2016-04-15 13:41 ` Takashi Iwai
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2016-03-16 7:43 UTC (permalink / raw)
To: tim; +Cc: alsa-devel
Hello Tim Blechmann,
The patch 02bec4904508: "ALSA: lx6464es - driver for the digigram
lx6464es interface" from Mar 24, 2009, leads to the following static
checker warning:
sound/pci/lx6464es/lx_core.c:647 lx_pipe_wait_for_state()
error: uninitialized symbol'current_state'.
sound/pci/lx6464es/lx_core.c
612 int lx_pipe_state(struct lx6464es *chip, u32 pipe, int is_capture, u16 *rstate)
613 {
614 int err;
615 u32 pipe_cmd = PIPE_INFO_TO_CMD(is_capture, pipe);
616
617 mutex_lock(&chip->msg_lock);
618 lx_message_init(&chip->rmh, CMD_0A_GET_PIPE_SPL_COUNT);
619
620 chip->rmh.cmd[0] |= pipe_cmd;
621
622 err = lx_message_send_atomic(chip, &chip->rmh);
lx_message_send_atomic() returns either negative error codes or a bit
mask.
623
624 if (err != 0)
^^^^^^^^
This code assumes that all non-zero values are errors which seems not
true since it could be a bit mask.
625 dev_err(chip->card->dev, "could not query pipe's state\n");
626 else
627 *rstate = (chip->rmh.stat[0] >> PSTATE_OFFSET) & 0x0F;
628
629 mutex_unlock(&chip->msg_lock);
630 return err;
631 }
632
633 static int lx_pipe_wait_for_state(struct lx6464es *chip, u32 pipe,
634 int is_capture, u16 state)
635 {
636 int i;
637
638 /* max 2*PCMOnlyGranularity = 2*1024 at 44100 = < 50 ms:
639 * timeout 50 ms */
640 for (i = 0; i != 50; ++i) {
641 u16 current_state;
642 int err = lx_pipe_state(chip, pipe, is_capture, ¤t_state);
643
644 if (err < 0)
^^^^^^^
This code assume that only negatives are errors.
645 return err;
646
647 if (current_state == state)
^^^^^^^^^^^^^^^^^^^^^^
This is potentially slightly buggy because we returned a bit mask so we
didn't initialize it. It not terribly harmful but the warning message
is annoying.
648 return 0;
649
650 mdelay(1);
651 }
652
653 return -ETIMEDOUT;
654 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: ALSA: lx6464es - driver for the digigram lx6464es interface
2016-03-16 7:43 ALSA: lx6464es - driver for the digigram lx6464es interface Dan Carpenter
@ 2016-04-15 13:41 ` Takashi Iwai
0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2016-04-15 13:41 UTC (permalink / raw)
To: Dan Carpenter; +Cc: tim, alsa-devel
On Wed, 16 Mar 2016 08:43:57 +0100,
Dan Carpenter wrote:
>
> Hello Tim Blechmann,
>
> The patch 02bec4904508: "ALSA: lx6464es - driver for the digigram
> lx6464es interface" from Mar 24, 2009, leads to the following static
> checker warning:
>
> sound/pci/lx6464es/lx_core.c:647 lx_pipe_wait_for_state()
> error: uninitialized symbol'current_state'.
>
> sound/pci/lx6464es/lx_core.c
> 612 int lx_pipe_state(struct lx6464es *chip, u32 pipe, int is_capture, u16 *rstate)
> 613 {
> 614 int err;
> 615 u32 pipe_cmd = PIPE_INFO_TO_CMD(is_capture, pipe);
> 616
> 617 mutex_lock(&chip->msg_lock);
> 618 lx_message_init(&chip->rmh, CMD_0A_GET_PIPE_SPL_COUNT);
> 619
> 620 chip->rmh.cmd[0] |= pipe_cmd;
> 621
> 622 err = lx_message_send_atomic(chip, &chip->rmh);
>
> lx_message_send_atomic() returns either negative error codes or a bit
> mask.
>
> 623
> 624 if (err != 0)
> ^^^^^^^^
>
> This code assumes that all non-zero values are errors which seems not
> true since it could be a bit mask.
>
> 625 dev_err(chip->card->dev, "could not query pipe's state\n");
> 626 else
> 627 *rstate = (chip->rmh.stat[0] >> PSTATE_OFFSET) & 0x0F;
> 628
> 629 mutex_unlock(&chip->msg_lock);
> 630 return err;
> 631 }
> 632
> 633 static int lx_pipe_wait_for_state(struct lx6464es *chip, u32 pipe,
> 634 int is_capture, u16 state)
> 635 {
> 636 int i;
> 637
> 638 /* max 2*PCMOnlyGranularity = 2*1024 at 44100 = < 50 ms:
> 639 * timeout 50 ms */
> 640 for (i = 0; i != 50; ++i) {
> 641 u16 current_state;
> 642 int err = lx_pipe_state(chip, pipe, is_capture, ¤t_state);
> 643
> 644 if (err < 0)
> ^^^^^^^
> This code assume that only negatives are errors.
>
> 645 return err;
> 646
> 647 if (current_state == state)
> ^^^^^^^^^^^^^^^^^^^^^^
> This is potentially slightly buggy because we returned a bit mask so we
> didn't initialize it. It not terribly harmful but the warning message
> is annoying.
>
> 648 return 0;
> 649
> 650 mdelay(1);
> 651 }
> 652
> 653 return -ETIMEDOUT;
> 654 }
>
> regards,
> dan carpenter
I queued the following fix. Thanks.
Takashi
-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: lx646es: Fix possible uninitialized variable reference
lx_pipe_state() checks the return value from lx_message_send_atomic()
and breaks the loop only when it's a negative value. However,
lx_message_send_atomic() may return a positive error code (as the
return code from the hardware), and then lx_pipe_state() tries to
compare the uninitialized current_state variable.
Fix this behavior by checking the positive non-zero error code as
well.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/pci/lx6464es/lx_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/lx6464es/lx_core.c b/sound/pci/lx6464es/lx_core.c
index f3d62020ef66..a80684bdc30d 100644
--- a/sound/pci/lx6464es/lx_core.c
+++ b/sound/pci/lx6464es/lx_core.c
@@ -644,7 +644,7 @@ static int lx_pipe_wait_for_state(struct lx6464es *chip, u32 pipe,
if (err < 0)
return err;
- if (current_state == state)
+ if (!err && current_state == state)
return 0;
mdelay(1);
--
2.8.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-04-15 13:41 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-16 7:43 ALSA: lx6464es - driver for the digigram lx6464es interface Dan Carpenter
2016-04-15 13:41 ` Takashi Iwai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.