Date: Tue, 10 Aug 2010 12:55:38 +0530 From: "Jayachandran C." <c.jayachandran@gmail.com> To: Neel Natu <neelnatu@gmail.com> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, Stefan Farfeleder <stefanf@freebsd.org>, src-committers@freebsd.org Subject: Re: svn commit: r211130 - head/libexec/rtld-elf/mips Message-ID: <AANLkTinkTQ25Dv6giE2HnWFrq7fuPReYFmwVb5oEy92s@mail.gmail.com> In-Reply-To: <AANLkTimwoLfMXrNb7NmG%2BHWonug1-asmNdAE9AcSNHkK@mail.gmail.com> References: <201008100515.o7A5FZZF017552@svn.freebsd.org> <20100810062829.GA1737@mole.fafoe.narf.at> <AANLkTimwoLfMXrNb7NmG%2BHWonug1-asmNdAE9AcSNHkK@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Aug 10, 2010 at 12:12 PM, Neel Natu <neelnatu@gmail.com> wrote: > Hi Stefan, > > On Mon, Aug 9, 2010 at 11:28 PM, Stefan Farfeleder <stefanf@freebsd.org> wrote: >> On Tue, Aug 10, 2010 at 05:15:35AM +0000, Neel Natu wrote: >>> Author: neel >>> Date: Tue Aug 10 05:15:35 2010 >>> New Revision: 211130 >>> URL: http://svn.freebsd.org/changeset/base/211130 >>> >>> Log: >>> Fix compilation error for 64-bit little endian build: >>> libexec/rtld-elf/mips/reloc.c:196: warning: right shift count >= width of type >>> >>> When the expression '(r_info) >> 32' was passed to bswap32() it was promptly >>> changed to '(uint32_t)(r_info) >> 32' which is not what we intended. >> >> Wouldn't it be better to fix the bswap32 macro instead? >> > > I think you are right. Can you take a look at the following patch instead? > > If I hear no objections, I will commit it tomorrow. > > Index: libexec/rtld-elf/mips/reloc.c > =================================================================== > --- libexec/rtld-elf/mips/reloc.c (revision 211131) > +++ libexec/rtld-elf/mips/reloc.c (working copy) > @@ -83,7 +83,7 @@ > #undef ELF_R_SYM > #undef ELF_R_TYPE > #define ELF_R_SYM(r_info) ((r_info) & 0xffffffff) > -#define ELF_R_TYPE(r_info) bswap32(((r_info) >> 32)) > +#define ELF_R_TYPE(r_info) bswap32((r_info) >> 32) > #endif > #else > #define ELF_R_NXTTYPE_64_P(r_type) (0) > Index: sys/sys/endian.h > =================================================================== > --- sys/sys/endian.h (revision 211131) > +++ sys/sys/endian.h (working copy) > @@ -56,9 +56,9 @@ > /* > * General byte order swapping functions. > */ > -#define bswap16(x) __bswap16(x) > -#define bswap32(x) __bswap32(x) > -#define bswap64(x) __bswap64(x) > +#define bswap16(x) __bswap16((x)) > +#define bswap32(x) __bswap32((x)) > +#define bswap64(x) __bswap64((x)) > I think there is a problem in sys/mips/include/_endian.h -- #define __bswap16(x) (__uint16_t)(__is_constant(x) ? \ __bswap16_const((__uint16_t)x) : __bswap16_var((__uint16_t)x)) #define __bswap32(x) (__uint32_t)(__is_constant(x) ? \ __bswap32_const((__uint32_t)x) : __bswap32_var((__uint32_t)x)) #define __bswap64(x) (__uint64_t)(__is_constant(x) ? \ __bswap64_const((__uint64_t)x) : __bswap64_var((__uint64_t)x)) -- I'm not sure why the cast is needed, but we should have a braces around x, unless I'm completely mistaken. JC.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTinkTQ25Dv6giE2HnWFrq7fuPReYFmwVb5oEy92s>