All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chase Southwood <chase.southwood@yahoo.com>
To: gregkh@linuxfoundation.org
Cc: abbotti@mev.co.uk, hsweeten@visionengravers.com,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
	Chase Southwood <chase.southwood@yahoo.com>
Subject: [PATCH v2] Staging: comedi: clean up conditional statement in addi_apci_3xxx.c
Date: Thu, 13 Feb 2014 19:02:26 -0600	[thread overview]
Message-ID: <1392339746-3901-1-git-send-email-chase.southwood@yahoo.com> (raw)
In-Reply-To: <1392262167-4073-1-git-send-email-chase.southwood@yahoo.com>

In this conditional statement, if (chan < 16), but the instruction passed
in data[0] is INSN_CONFIG_DIO_QUERY, the function does not return early,
but the else-branch does not get executed either.  As a result, mask
would be used uninitialized in the next line.  We want
comedi_dio_insn_config() to use a chan_mask based on the chanspec for the
INSN_CONFIG_DIO_QUERY instruction, so mask should be initialized to 0.
Then, if instead the instruction is INSN_CONFIG_DIO_{INPUT,OUTPUT}, we
return an error if (chan < 16) as these are invalid instructions for
ports 0 and 1, or update the mask otherwise, so all the io_bits are
modified for port 2.  This ensures that mask is always initialized by the
time it is used.

Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
---

2: Addressed all of the comments provided by Hartley regarding correct
structure of this patch.

Hopefully everything looks better!

 drivers/staging/comedi/drivers/addi_apci_3xxx.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_3xxx.c b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
index ceadf8e..1f3b668 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3xxx.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
@@ -680,7 +680,7 @@ static int apci3xxx_dio_insn_config(struct comedi_device *dev,
 				    unsigned int *data)
 {
 	unsigned int chan = CR_CHAN(insn->chanspec);
-	unsigned int mask;
+	unsigned int mask = 0;
 	int ret;
 
 	/*
@@ -688,12 +688,13 @@ static int apci3xxx_dio_insn_config(struct comedi_device *dev,
 	 * Port 1 (channels 8-15) are always outputs
 	 * Port 2 (channels 16-23) are programmable i/o
 	 */
-	if (chan < 16) {
-		if (data[0] != INSN_CONFIG_DIO_QUERY)
+	if (data[0] != INSN_CONFIG_DIO_QUERY) {
+		/* ignore all other instructions for ports 0 and 1 */
+		if (chan < 16)
 			return -EINVAL;
-	} else {
-		/* changing any channel in port 2 changes the entire port */
-		mask = 0xff0000;
+		else
+			/* changing any channel in port 2 changes the entire port */
+			mask = 0xff0000;
 	}
 
 	ret = comedi_dio_insn_config(dev, s, insn, data, mask);
-- 
1.8.5.3


  parent reply	other threads:[~2014-02-14  1:02 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-13  3:29 [PATCH] Staging: comedi: clean up conditional statement in addi_apci_3xxx.c Chase Southwood
2014-02-13 10:08 ` Ian Abbott
2014-02-14 11:52   ` Ian Abbott
2014-02-13 18:25 ` Hartley Sweeten
2014-02-14 11:49   ` Ian Abbott
2014-02-14 17:54     ` Hartley Sweeten
2014-02-14  1:02 ` Chase Southwood [this message]
2014-02-14 11:54   ` [PATCH v2] " Ian Abbott

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1392339746-3901-1-git-send-email-chase.southwood@yahoo.com \
    --to=chase.southwood@yahoo.com \
    --cc=abbotti@mev.co.uk \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hsweeten@visionengravers.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.