| Message ID | 1423517995-28251-1-git-send-email-jan.vesely@rutgers.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/09/2015 01:39 PM, Jan Vesely wrote: > This is more consistent with the rest, and avoids potential undefined > behavior (signed overflow) in drmRandom() > > Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu> > --- > xf86drmRandom.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/xf86drmRandom.c b/xf86drmRandom.c > index ecab9e2..a084b86 100644 > --- a/xf86drmRandom.c > +++ b/xf86drmRandom.c > @@ -98,7 +98,7 @@ typedef struct RandomState { > unsigned long q; /* m div a */ > unsigned long r; /* m mod a */ > unsigned long check; > - long seed; > + unsigned long seed; > } RandomState; > > #if RANDOM_MAIN > @@ -147,13 +147,13 @@ int drmRandomDestroy(void *state) > unsigned long drmRandom(void *state) > { > RandomState *s = (RandomState *)state; > - long hi; > - long lo; > + unsigned long hi; > + unsigned long lo; > > hi = s->seed / s->q; > lo = s->seed % s->q; > s->seed = s->a * lo - s->r * hi; > - if (s->seed <= 0) s->seed += s->m; > + if ((s->a * lo) <= (s->r * hi)) s->seed += s->m; This seems like an unrelated change. > > return s->seed; > } > @@ -166,7 +166,7 @@ double drmRandomDouble(void *state) > } > > #if RANDOM_MAIN > -static void check_period(long seed) > +static void check_period(unsigned long seed) > { > unsigned long count = 0; > unsigned long initial; > @@ -178,7 +178,7 @@ static void check_period(long seed) > while (initial != drmRandom(state)) { > if (!++count) break; > } > - printf("With seed of %10ld, period = %10lu (0x%08lx)\n", > + printf("With seed of %10lu, period = %10lu (0x%08lx)\n", > seed, count, count); > drmRandomDestroy(state); > } > @@ -195,7 +195,7 @@ int main(void) > } > printf("After 10000 iterations: %lu (%lu expected): %s\n", > rand, state->check, > - rand - state->check ? "*INCORRECT*" : "CORRECT"); > + rand != state->check ? "*INCORRECT*" : "CORRECT"); So does this. > drmRandomDestroy(state); > > printf("Checking periods...\n"); >
On Mon, 2015-02-09 at 15:11 -0800, Ian Romanick wrote: > On 02/09/2015 01:39 PM, Jan Vesely wrote: > > This is more consistent with the rest, and avoids potential undefined > > behavior (signed overflow) in drmRandom() > > > > Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu> > > --- > > xf86drmRandom.c | 14 +++++++------- > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/xf86drmRandom.c b/xf86drmRandom.c > > index ecab9e2..a084b86 100644 > > --- a/xf86drmRandom.c > > +++ b/xf86drmRandom.c > > @@ -98,7 +98,7 @@ typedef struct RandomState { > > unsigned long q; /* m div a */ > > unsigned long r; /* m mod a */ > > unsigned long check; > > - long seed; > > + unsigned long seed; > > } RandomState; > > > > #if RANDOM_MAIN > > @@ -147,13 +147,13 @@ int drmRandomDestroy(void *state) > > unsigned long drmRandom(void *state) > > { > > RandomState *s = (RandomState *)state; > > - long hi; > > - long lo; > > + unsigned long hi; > > + unsigned long lo; > > > > hi = s->seed / s->q; > > lo = s->seed % s->q; > > s->seed = s->a * lo - s->r * hi; > > - if (s->seed <= 0) s->seed += s->m; > > + if ((s->a * lo) <= (s->r * hi)) s->seed += s->m; > > This seems like an unrelated change. This change is necessary. since s->seed is unsigned now it cannot be < 0. also the result of s->seed op s->q is unsigned. > > > > > return s->seed; > > } > > @@ -166,7 +166,7 @@ double drmRandomDouble(void *state) > > } > > > > #if RANDOM_MAIN > > -static void check_period(long seed) > > +static void check_period(unsigned long seed) > > { > > unsigned long count = 0; > > unsigned long initial; > > @@ -178,7 +178,7 @@ static void check_period(long seed) > > while (initial != drmRandom(state)) { > > if (!++count) break; > > } > > - printf("With seed of %10ld, period = %10lu (0x%08lx)\n", > > + printf("With seed of %10lu, period = %10lu (0x%08lx)\n", > > seed, count, count); > > drmRandomDestroy(state); > > } > > @@ -195,7 +195,7 @@ int main(void) > > } > > printf("After 10000 iterations: %lu (%lu expected): %s\n", > > rand, state->check, > > - rand - state->check ? "*INCORRECT*" : "CORRECT"); > > + rand != state->check ? "*INCORRECT*" : "CORRECT"); > > So does this. I'll drop this one in v2, it shouldn't have been included > > > drmRandomDestroy(state); > > > > printf("Checking periods...\n"); > > >
diff --git a/xf86drmRandom.c b/xf86drmRandom.c index ecab9e2..a084b86 100644 --- a/xf86drmRandom.c +++ b/xf86drmRandom.c @@ -98,7 +98,7 @@ typedef struct RandomState { unsigned long q; /* m div a */ unsigned long r; /* m mod a */ unsigned long check; - long seed; + unsigned long seed; } RandomState; #if RANDOM_MAIN @@ -147,13 +147,13 @@ int drmRandomDestroy(void *state) unsigned long drmRandom(void *state) { RandomState *s = (RandomState *)state; - long hi; - long lo; + unsigned long hi; + unsigned long lo; hi = s->seed / s->q; lo = s->seed % s->q; s->seed = s->a * lo - s->r * hi; - if (s->seed <= 0) s->seed += s->m; + if ((s->a * lo) <= (s->r * hi)) s->seed += s->m; return s->seed; } @@ -166,7 +166,7 @@ double drmRandomDouble(void *state) } #if RANDOM_MAIN -static void check_period(long seed) +static void check_period(unsigned long seed) { unsigned long count = 0; unsigned long initial; @@ -178,7 +178,7 @@ static void check_period(long seed) while (initial != drmRandom(state)) { if (!++count) break; } - printf("With seed of %10ld, period = %10lu (0x%08lx)\n", + printf("With seed of %10lu, period = %10lu (0x%08lx)\n", seed, count, count); drmRandomDestroy(state); } @@ -195,7 +195,7 @@ int main(void) } printf("After 10000 iterations: %lu (%lu expected): %s\n", rand, state->check, - rand - state->check ? "*INCORRECT*" : "CORRECT"); + rand != state->check ? "*INCORRECT*" : "CORRECT"); drmRandomDestroy(state); printf("Checking periods...\n");
This is more consistent with the rest, and avoids potential undefined behavior (signed overflow) in drmRandom() Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu> --- xf86drmRandom.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)