Re: [PATCH] Fix typing of the bit twiddling done in _dl_setup_stack_chk_guard.

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

On Sun, 11 Mar 2012 19:47:51 -0700 (PDT), David Miller <davem@davemloft.net> wrote:
> Committed to master.
> 
> 	* sysdeps/unix/sysv/linux/dl-osinfo.h (_dl_setup_stack_chk_guard):
> 	Fix masking out of the most significant byte of random value used.

> --- a/sysdeps/unix/sysv/linux/dl-osinfo.h
> +++ b/sysdeps/unix/sysv/linux/dl-osinfo.h
> @@ -95,9 +95,9 @@ _dl_setup_stack_chk_guard (void *dl_random)
>  	 directly and not use the kernel-provided data to seed a PRNG.  */
>        memcpy (ret.bytes, dl_random, sizeof (ret));
>  #if BYTE_ORDER == LITTLE_ENDIAN
> -      ret.num &= ~0xff;
> +      ret.num &= ~(uintptr_t)0xff;
>  #elif BYTE_ORDER == BIG_ENDIAN
> -      ret.num &= ~(0xff << (8 * (sizeof (ret) - 1)));
> +      ret.num &= ~((uintptr_t)0xff << (8 * (sizeof (ret) - 1)));
>  #else
>  # error "BYTE_ORDER unknown"
>  #endif

The same code exists in sysdeps/generic/dl-osinfo.h -- and the same
reasoning applies there.

I propose that we set a policy (to try) to fix *all* occurrences at the
same time (for such simple cases).  For the sake of keeping the glibc
code uniformly and comprehensible -- the goal should (typically) be to
improve glibc generally, and not just one variant of something.  Just
doing such a change in one place and then hoping that someone else will
eventually fix the other(s) takes up more of the community's human
resources compared to changing both at the same time, and the latter
approach will also not scatter the commit history as much.

Of course, it might simply be that you have not noticed the other
dl-osinfo.h file, but that would/should then have been pointed out when
the proposed patch gets reviewed.

And, as I already said in another email
(<http://sourceware.org/ml/libc-alpha/2012-03/msg00222.html>; no comments
received), I don't think such a change requires testing for all affected
architectures (whatever that would mean -- testing is only one specific
sample anyway).  In my book, for such cases, peer code review as well as
taking one or two testing samples is enough.

Now any comments?


GrÃÃe,
 Thomas

Attachment: pgp00000.pgp
Description: PGP signature