Re: [PATCH] DVB: negative internal->sub_range won't get noticed

[Manu Abraham]

On Mon, 19 Jan 2009 00:35:47 +0100 Roel Kluin <roel.kluin@xxxxxxxxx>
wrote:

> internal->sub_range is unsigned, a negative won't get noticed.
>
> Signed-off-by: Roel Kluin <roel.kluin@xxxxxxxxx>
> ---
> diff --git a/drivers/media/dvb/frontends/stb0899_algo.c
b/drivers/media/dvb/frontends/stb0899_algo.c
> index 83dc7e1..2ea32da 100644
> --- a/drivers/media/dvb/frontends/stb0899_algo.c
> +++ b/drivers/media/dvb/frontends/stb0899_algo.c
> @@ -464,13 +464,14 @@ static void next_sub_range(struct
stb0899_state *state)
>
>  	if (internal->sub_dir > 0) {
>  		old_sub_range = internal->sub_range;
> -		internal->sub_range = MIN((internal->srch_range / 2) -
> +		if (internal->tuner_offst + internal->sub_range / 2 >=
> +				internal->srch_range / 2)
> +			internal->sub_range = 0;
> +		else
> +			internal->sub_range = MIN((internal->srch_range / 2) -
>  					  (internal->tuner_offst + internal->sub_range / 2),
>  					   internal->sub_range);
>
> -		if (internal->sub_range < 0)
> -			internal->sub_range = 0;
> -
>  		internal->tuner_offst += (old_sub_range + internal->sub_range) / 2;
>  	}

> I hope someone understands that function :(


I guess the relevant people might. Sending it to wrong people and to
a wrong mailing list, doesn't help much i guess.


That function does a step through a "next slice" of the RF carrier
based on a RF center frequency, which is used for carrier acquisition.


> Do we actually need that test at all?  Perhaps it has never
triggered?


Ranges can go negative, from the current offset to the left anything
is negative, to the right it is positive. There of course is a bug
that the variable is not signed.

I have pushed out a fix out here at
http://jusst.de/hg/v4l-dvb/rev/368dc6078295

including your comment on the macros. Thanks for pointing the bug.

Regards,
Manu




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/