Re: [libdrm][PATCH 1/2] random: Use unsigned long for seed

[Jan Vesely <jan.vesely@rutgers.edu>]

[-- Attachment #1.1: Type: text/plain, Size: 2655 bytes --]

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");
> > 
> 

-- 
Jan Vesely <jan.vesely@rutgers.edu>

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel