[PATCH] staging: comedi: range: tidy up comedi_check_chanlist()

Ian Abbott abbotti at mev.co.uk
Thu Jun 27 13:27:19 UTC 2013


[Originally posted 2013-06-25.  Reposting to new driverdev-devel list.]

On 2013-06-25 01:11, H Hartley Sweeten wrote:
> The only difference in the if() and else if() check of the chanlist
> is the source of the range table length. Consolidate the checks to
> make the function a bit more concise.
>
> Signed-off-by: H Hartley Sweeten <hsweeten at visionengravers.com>
> Cc: Ian Abbott <abbotti at mev.co.uk>
> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> ---
>   drivers/staging/comedi/range.c | 41 ++++++++++++++++++-----------------------
>   1 file changed, 18 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/staging/comedi/range.c b/drivers/staging/comedi/range.c
> index 1f20332..fbc9d41 100644
> --- a/drivers/staging/comedi/range.c
> +++ b/drivers/staging/comedi/range.c
> @@ -127,38 +127,33 @@ static int aref_invalid(struct comedi_subdevice *s, unsigned int chanspec)
>   	return 1;
>   }
>
> -/*
> -   This function checks each element in a channel/gain list to make
> -   make sure it is valid.
> +/**
> + * comedi_check_chanlist() - Validate each element in a chanlist.
> + * @s: comedi_subdevice struct
> + * @n: number of elements in the chanlist
> + * @chanlist: the chanlist to validate
>   */
>   int comedi_check_chanlist(struct comedi_subdevice *s, int n,
>   			  unsigned int *chanlist)
>   {
>   	struct comedi_device *dev = s->device;
> -	int i;
> -	int chan;
> +	unsigned int chanspec;
> +	int chan, range_len, i;
>
> -	if (s->range_table) {
> -		for (i = 0; i < n; i++)
> -			if (CR_CHAN(chanlist[i]) >= s->n_chan ||
> -			    CR_RANGE(chanlist[i]) >= s->range_table->length
> -			    || aref_invalid(s, chanlist[i])) {
> -				dev_warn(dev->class_dev,
> -					 "bad chanlist[%d]=0x%08x in_chan=%d range length=%d\n",
> -					 i, chanlist[i], s->n_chan,
> -					 s->range_table->length);
> -				return -EINVAL;
> -			}
> -	} else if (s->range_table_list) {
> +	if (s->range_table || s->range_table_list) {
>   		for (i = 0; i < n; i++) {
> -			chan = CR_CHAN(chanlist[i]);
> +			chanspec = chanlist[i];
> +			chan = CR_CHAN(chanspec);
> +			if (s->range_table)
> +				range_len = s->range_table->length;
> +			else
> +				range_len = s->range_table_list[chan]->length;

s->range_table_list[chan] may be an invalid here, as chan has not been 
checked for validity yet.

>   			if (chan >= s->n_chan ||
> -			    CR_RANGE(chanlist[i]) >=
> -			    s->range_table_list[chan]->length
> -			    || aref_invalid(s, chanlist[i])) {
> +			    CR_RANGE(chanspec) >= range_len ||
> +			    aref_invalid(s, chanspec)) {
>   				dev_warn(dev->class_dev,
> -					 "bad chanlist[%d]=0x%08x\n",
> -					 i, chanlist[i]);
> +					 "bad chanlist[%d]=0x%08x chan=%d range length=%d\n",
> +					 i, chanspec, chan, range_len);
>   				return -EINVAL;
>   			}
>   		}
>


-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-


More information about the devel mailing list