Re: [PATCH] review request: implementing DW_AT_endianity

[Peeter Joot <peeter.joot@gmail.com>]

I'm still holding the patch for submission.  I'd handled all the reviewer
comments I received, implemented the tests suggestion by the reviewers, and
verified that I introduced no test regressions.  Also, the gcc side issues
I noticed testing this are already fixed in the latest version (trunk, but
perhaps one of the releases now too).

Unfortunately, the FSF attribution required for me to submit this formally
on behalf of the company I work for has been delayed repeatedly.  The last
I heard from the company lawyer who was arranging the attribution is that
there's just a few more i's and t's to dot and cross, but it should be done
soon.

Let me try your differs from arch suggestion.  With respect to the gdbarch
parameter, I'm not sure, as it's been a while since I worked on this, and
will have to inspect what I was doing before I can answer.

Peeter

--
Peeter

Note that my preferred email address is now: peeterjoot@protonmail.com

On Thu, Feb 22, 2018 at 12:20 PM, Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Peeter" == Peeter Joot <peeter.joot@gmail.com> writes:
>
> I happened to run across this patch today.
>
> What's the status of it?
>
> Peeter> +enum bfd_endian
> Peeter> +type_byte_order (struct gdbarch * gdbarch, struct type *type)
> Peeter> +{
> Peeter> +  if (TYPE_ENDIANITY_BIG (type))
> Peeter> +    return BFD_ENDIAN_BIG;
> Peeter> +  else if (TYPE_ENDIANITY_LITTLE (type))
> Peeter> +    return BFD_ENDIAN_LITTLE;
> Peeter> +  if (!gdbarch)
> Peeter> +    gdbarch = get_type_arch (type);
> Peeter> +  return gdbarch_byte_order (gdbarch);
>
> Does it ever make sense to call type_byte_order with a gdbarch other
> than the type's gdbarch?  I would assume not but I'm not really sure.
> What would this mean?
>
> Anyway, if it doesn't make sense, then I'd suggest just removing the
> gdbarch parameter.
>
> Peeter> +  unsigned int flag_endianity_big : 1;
> Peeter> +  unsigned int flag_endianity_little : 1;
>
> It also seems to me that perhaps only a single bit is needed --
> something like:
>
>    unsigned int flag_endian_differs_from_arch : 1;
>
> Then type_byte_order could do:
>
>   enum bfd_endian endian = gdbarch_byte_order (...);
>   if (blah blah flag_endian_differs_from_arch)
>     endian = (endian == BFD_ENDIAN_LITTLE) ? BFD_ENDIAN_BIG :
> BFD_ENDIAN_LITTLE;
>
> Of course this only makes sense if the arch endianness can't change
> somehow, and if only the type's arch can be used to get the endianness.
>
> Tom
>