This is the mail archive of the newlib@sources.redhat.com mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: recent breakage on mips: hash.c?


cgd@broadcom.com wrote:
> 
> At Fri, 21 Jun 2002 15:49:10 -0400, J. Johnston wrote:
> > > > Looks like this would normally be defined by
> > > > libc/include/sys/param.h because of __IEEE_{BIG,LITTLE}_ENDIAN.
> > > >
> > > > In turn, those are defined in one of:
> > > >
> > > >         ./libc/include/machine/ieeefp.h
> > > >         ./libc/include/sys/config.h
> > > >
> > > > it looks like MIPS currently only has the macros defined in the
> > > > former.  The latter has the comment:
> > [ ... ]
> > > I have a patch which I am testing as we speak.  I have placed a default setting in
> > > sys/param.h based on whether sys/config.h sets BYTE_ORDER or not.  The same default
> > > setting goes in hash.h in libc/search.  Both are protected.  This seems to remove the
> > > problem and avoid conflict with Cygwin that defines BYTE_ORDER or the other systems that
> > > provide sys/param.h.
> > >
> > > I will repost when my testing is completed and the patch is applied.
> > >
> >
> > Patches applied.
> 
> So, I just got a chance to look at this patch, and looked at the
> related code some more.
> 
> yes, the generic param.h now defines BYTE_ORDER depending on whether
> __IEEE_LITTLE_ENDIAN is defined, but, to my mind, there are some
> issues:
> 
> (1) I'd still like to understand the need for duplication between
>     ieeefp.h and config.h to begin with...  8-)
> 

Historical mess :)  One of those todo's that always falls off the bottom of my list.  I'll finally look at including <machine/ieeefp.h> from
<sys/config.h> and removing the redundant bits after
this patch is evaluated/checked in.

> (2) mips, in particular, seems to still need changes to sys/param.h to
>     have the right thing happen.  (i.e., you fixed a problem, but it
>     wasn't the one I reported, and didn't give me a clue whether my
>     proposed solution to the original problem was correct.
>

It might have been clearer had I not forgotten to also copy over the MIPS stuff like you
requested. :(
 
> (3) the change you made even further muddies the water when config.h
>     erroneously fails to set either of __IEEE_{BIG,LITTLE}_ENDIAN:
>     now, you get a setting which causes the code to compile (probably)
>     but may not be correct.
> 

The check should actually be in <sys/config.h> like it is for <machine/ieeefp.h> so
all code doesn't constantly have to be checking this.  This is minor change.

> (4) I notice that the libc/search code checks for the define
>     _IEEE_LITTLE_ENDIAN (not one leaing underscore rather than 2!),
>     which doens't seem correct, esp. in light of the fact that
>     BYTE_ORDER is the "right thing" to use, right?
> 

Good catch on the single underscore.  BYTE_ORDER would be the consistent thing
to use since it used elsewhere.

> (5) the DB code (at least, if it's the same DB code I'm used to 8-)
>     uses values for BIG_ENDIAN and LITTLE_ENDIAN _in its on-disk data
>     structures_, and it's not good to have those differ from system to
>     system.  It's not immediately clear to me whether any of the
>     libc/search bits will end up anyplace but in memory.
>

Reasonable.

> Based on those, I'd propose the following patch.  I've not yet tested
> it (in progress for mips-elf and a few others), but i'd like some
> feedback on it anyway.
> 
> Specifically it does the following:
> 
> * addresses (2), (4).
> 
> * errors out instead of silently doing the possibly-wrong thing for
>   (3).  Note that this will almost certainly break for sysvi386, but i
>   believe it was not previously correct: its param.h doesn't define
>   BYTE_ORDER, but the default for the hash code was big-endian...  So,
>   now it'll actually error out rather than compiling w/ an
>   ... unexpected setting.
> 
> * addresses (5) by renaming the byte-order-related constants that DB
>   uses, in order to preserve their intended uses.
> 

Looks good Chris.  Repost when you have finished your testing.

-- Jeff J.
    
> cgd
> ==============================================================================
> 2002-06-25  Chris Demetriou  <cgd@broadcom.com>
> 
>         * libc/include/sys/config.h (__IEEE_LITTLE_ENDIAN)
>         (__IEEE_BIG_ENDIAN): Define appropriately for MIPS.
>         * libc/include/sys/param.h (BYTE_ORDER): If BYTE_ORDER is
>         not defined, verify that one of __IEEE_LITTLE_ENDIAN and
>         __IEEE_BIG_ENDIAN are set.
>         * libc/search/hash.h (DB_BYTE_ORDER, DB_BIG_ENDIAN)
>         (DB_LITTLE_ENDIAN): New defines.
>         * libc/search/hash.c: Replace all incorrect checks for
>         _IEEE_LITTLE_ENDIAN with tests of BYTE_ORDER, and all uses of
>         BYTE_ORDER, LITTLE_ENDIAN, and BIG_ENDIAN with DB_* versions.
>         * libc/search/hash_page.c: Likewise.
> 
> Index: libc/include/sys/config.h
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/include/sys/config.h,v
> retrieving revision 1.20
> diff -u -p -r1.20 config.h
> --- libc/include/sys/config.h   21 Jun 2002 18:40:48 -0000      1.20
> +++ libc/include/sys/config.h   25 Jun 2002 19:41:02 -0000
> @@ -82,6 +82,13 @@
>  #define __IEEE_LITTLE_ENDIAN
>  #endif
> 
> +#ifdef __MIPSEL__
> +#define __IEEE_LITTLE_ENDIAN
> +#endif
> +#ifdef __MIPSEB__
> +#define __IEEE_BIG_ENDIAN
> +#endif
> +
>  #ifdef __MMIX__
>  #define __IEEE_BIG_ENDIAN
>  #endif
> Index: libc/include/sys/param.h
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/include/sys/param.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 param.h
> --- libc/include/sys/param.h    21 Jun 2002 18:15:53 -0000      1.3
> +++ libc/include/sys/param.h    25 Jun 2002 19:41:02 -0000
> @@ -19,11 +19,18 @@
>  # define PATHSIZE (1024)
> 
>  #ifndef BYTE_ORDER
> +
> +/* Check that sys/config.h defined the endianness for us.  */
> +#if !defined(__IEEE_BIG_ENDIAN) && !defined(__IEEE_LITTLE_ENDIAN)
> +#error sys/config.h must define one of __IEEE_BIG_ENDIAN and __IEEE_LITTLE_ENDIAN
> +#endif
> +
>  #ifdef __IEEE_LITTLE_ENDIAN
>  #define BYTE_ORDER LITTLE_ENDIAN
>  #else
>  #define BYTE_ORDER BIG_ENDIAN
>  #endif
> -#endif
> +
> +#endif /* BYTE_ORDER */
> 
>  #endif
> Index: libc/search/hash.c
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/search/hash.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 hash.c
> --- libc/search/hash.c  24 Jun 2002 23:05:08 -0000      1.2
> +++ libc/search/hash.c  25 Jun 2002 19:41:03 -0000
> @@ -72,7 +72,7 @@ static int   hash_sync(const DB *, __uin
>  static int   hdestroy(HTAB *);
>  static HTAB *init_hash(HTAB *, const char *, HASHINFO *);
>  static int   init_htab(HTAB *, int);
> -#ifdef _IEEE_LITTLE_ENDIAN
> +#if (BYTE_ORDER == LITTLE_ENDIAN)
>  static void  swap_header(HTAB *);
>  static void  swap_header_copy(HASHHDR *, HASHHDR *);
>  #endif
> @@ -156,7 +156,7 @@ __hash_open(file, flags, mode, info, dfl
>                         hashp->hash = __default_hash;
> 
>                 hdrsize = read(hashp->fp, &hashp->hdr, sizeof(HASHHDR));
> -#ifdef _IEEE_LITTLE_ENDIAN
> +#if (BYTE_ORDER == LITTLE_ENDIAN)
>                 swap_header(hashp);
>  #endif
>                 if (hdrsize == -1)
> @@ -299,7 +299,7 @@ init_hash(hashp, file, info)
> 
>         nelem = 1;
>         hashp->NKEYS = 0;
> -       hashp->LORDER = BYTE_ORDER;
> +       hashp->LORDER = DB_BYTE_ORDER;
>         hashp->BSIZE = DEF_BUCKET_SIZE;
>         hashp->BSHIFT = DEF_BUCKET_SHIFT;
>         hashp->SGSIZE = DEF_SEGSIZE;
> @@ -335,8 +335,8 @@ init_hash(hashp, file, info)
>                 if (info->nelem)
>                         nelem = info->nelem;
>                 if (info->lorder) {
> -                       if (info->lorder != BIG_ENDIAN &&
> -                           info->lorder != LITTLE_ENDIAN) {
> +                       if (info->lorder != DB_BIG_ENDIAN &&
> +                           info->lorder != DB_LITTLE_ENDIAN) {
>                                 errno = EINVAL;
>                                 return (NULL);
>                         }
> @@ -495,7 +495,7 @@ flush_meta(hashp)
>         HTAB *hashp;
>  {
>         HASHHDR *whdrp;
> -#ifdef _IEEE_LITTLE_ENDIAN
> +#if (BYTE_ORDER == LITTLE_ENDIAN)
>         HASHHDR whdr;
>  #endif
>         int fp, i, wsize;
> @@ -508,7 +508,7 @@ flush_meta(hashp)
> 
>         fp = hashp->fp;
>         whdrp = &hashp->hdr;
> -#ifdef _IEEE_LITTLE_ENDIAN
> +#if (BYTE_ORDER == LITTLE_ENDIAN)
>         whdrp = &whdr;
>         swap_header_copy(&hashp->hdr, whdrp);
>  #endif
> @@ -941,7 +941,7 @@ alloc_segs(hashp, nsegs)
>         return (0);
>  }
> 
> -#ifdef _IEEE_LITTLE_ENDIAN
> +#if (BYTE_ORDER == LITTLE_ENDIAN)
>  /*
>   * Hashp->hdr needs to be byteswapped.
>   */
> Index: libc/search/hash.h
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/search/hash.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 hash.h
> --- libc/search/hash.h  21 Jun 2002 19:09:50 -0000      1.3
> +++ libc/search/hash.h  25 Jun 2002 19:41:03 -0000
> @@ -39,18 +39,18 @@
> 
>  #include <sys/param.h>
> 
> +/* Check that newlib understands the byte order of its target system.  */
>  #ifndef BYTE_ORDER
> -#ifndef LITTLE_ENDIAN
> -#define LITTLE_ENDIAN 1234
> +#error BYTE_ORDER not defined by sys/param.h
>  #endif
> -#ifndef BIG_ENDIAN
> -#define BIG_ENDIAN 4321
> -#endif
> -#ifdef __IEEE_LITTLE_ENDIAN
> -#define BYTE_ORDER LITTLE_ENDIAN
> +
> +/* Define DB endianness constants based on target endianness.  */
> +#define DB_LITTLE_ENDIAN 1234
> +#define DB_BIG_ENDIAN 4321
> +#if (BYTE_ORDER == LITTLE_ENDIAN)
> +#define DB_BYTE_ORDER DB_LITTLE_ENDIAN
>  #else
> -#define BYTE_ORDER BIG_ENDIAN
> -#endif
> +#define DB_BYTE_ORDER DB_BIG_ENDIAN
>  #endif
> 
>  /* Operations */
> Index: libc/search/hash_page.c
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/search/hash_page.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 hash_page.c
> --- libc/search/hash_page.c     24 Jun 2002 23:05:08 -0000      1.2
> +++ libc/search/hash_page.c     25 Jun 2002 19:41:03 -0000
> @@ -552,7 +552,7 @@ __get_page(hashp, p, bucket, is_bucket,
>         if (!is_bitmap && !bp[0]) {
>                 PAGE_INIT(p);
>         } else
> -               if (hashp->LORDER != BYTE_ORDER) {
> +               if (hashp->LORDER != DB_BYTE_ORDER) {
>                         int i, max;
> 
>                         if (is_bitmap) {
> @@ -591,7 +591,7 @@ __put_page(hashp, p, bucket, is_bucket,
>                 return (-1);
>         fd = hashp->fp;
> 
> -       if (hashp->LORDER != BYTE_ORDER) {
> +       if (hashp->LORDER != DB_BYTE_ORDER) {
>                 int i;
>                 int max;
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]