* Re: [PATCH] DVB: negative internal->sub_range won't get noticed
@ 2009-01-27 18:40 Manu Abraham
0 siblings, 0 replies; 5+ messages in thread
From: Manu Abraham @ 2009-01-27 18:40 UTC (permalink / raw)
To: akpm; +Cc: roel.kluin, linux-kernel
On Mon, 19 Jan 2009 00:35:47 +0100 Roel Kluin <roel.kluin@gmail.com>
wrote:
> internal->sub_range is unsigned, a negative won't get noticed.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] DVB: negative internal->sub_range won't get noticed
2009-01-18 23:35 Roel Kluin
@ 2009-01-27 6:40 ` Andrew Morton
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2009-01-27 6:40 UTC (permalink / raw)
To: Roel Kluin; +Cc: mchehab, video4linux-list, lkml
On Mon, 19 Jan 2009 00:35:47 +0100 Roel Kluin <roel.kluin@gmail.com> wrote:
> internal->sub_range is unsigned, a negative won't get noticed.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> 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 :(
Do we actually need that test at all? Perhaps it has never triggered?
Perhaps values in the 0x80000000 - 0xffffffff are actually OK?
This driver has managed to get itself a secret private version of the
min(), max() and abs() macros. They're buggy - they reference their
argument multiple times. The driver should be converted to use the
kernel.h versions.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] DVB: negative internal->sub_range won't get noticed
@ 2009-01-27 6:40 ` Andrew Morton
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2009-01-27 6:40 UTC (permalink / raw)
To: Roel Kluin; +Cc: video4linux-list, mchehab, lkml
On Mon, 19 Jan 2009 00:35:47 +0100 Roel Kluin <roel.kluin@gmail.com> wrote:
> internal->sub_range is unsigned, a negative won't get noticed.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> 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 :(
Do we actually need that test at all? Perhaps it has never triggered?
Perhaps values in the 0x80000000 - 0xffffffff are actually OK?
This driver has managed to get itself a secret private version of the
min(), max() and abs() macros. They're buggy - they reference their
argument multiple times. The driver should be converted to use the
kernel.h versions.
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] DVB: negative internal->sub_range won't get noticed
@ 2009-01-18 23:35 Roel Kluin
2009-01-27 6:40 ` Andrew Morton
0 siblings, 1 reply; 5+ messages in thread
From: Roel Kluin @ 2009-01-18 23:35 UTC (permalink / raw)
To: mchehab; +Cc: video4linux-list, lkml
internal->sub_range is unsigned, a negative won't get noticed.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
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;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] DVB: negative internal->sub_range won't get noticed
@ 2009-01-18 23:31 Roel Kluin
0 siblings, 0 replies; 5+ messages in thread
From: Roel Kluin @ 2009-01-18 23:31 UTC (permalink / raw)
To: linux-media; +Cc: linux-dvb
internal->sub_range is unsigned, a negative won't get noticed.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
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;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-01-27 18:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-01-27 18:40 [PATCH] DVB: negative internal->sub_range won't get noticed Manu Abraham
-- strict thread matches above, loose matches on Subject: below --
2009-01-18 23:35 Roel Kluin
2009-01-27 6:40 ` Andrew Morton
2009-01-27 6:40 ` Andrew Morton
2009-01-18 23:31 Roel Kluin
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.