Re: [PATCH 2/3] Consistently use BFD's time

["Christian Biesinger via gdb-patches" <gdb-patches@sourceware.org>]

On Thu, Jan 16, 2020 at 3:37 PM Pedro Alves <palves@redhat.com> wrote:
>
> On 1/15/20 4:07 PM, Eli Zaretskii wrote:
> >> From: Tom Tromey <tromey@adacore.com>
> >> Cc: Tom Tromey <tromey@adacore.com>
> >> Date: Tue, 14 Jan 2020 14:09:55 -0700
> >>
> >> gdb uses the gnulib stat, while BFD does not.  This can lead to
> >> inconsistencies between the two, because the gnulib stat adjusts for
> >> timezones.
> >
> > There's one more potential issue with Gnulib's replacement of 'fstat':
> > it also replaces the definition of 'struct stat', and it does that in
> > a way that might yield incompatibility between the definition on
> > <sys/stat.h> the system header and Gnulib's sys/stat.h replacement.
> > If gdb_bfd.c uses the Gnulib definition of 'struct stat' (as I think
> > we do everywhere in gdb/), then this replacement might create problems
> > on MinGW similar to those I reported to the Gnulib list (see the URL I
> > cited in an earlier message), because bfd_stat uses an incompatible
> > definition of 'struct stat'.
> >
> > Of course, given that the Gnulib developers rejected my request not to
> > override the system definition of 'struct stat', GDB could also ignore
> > those problems, accepting their judgment.
>
> I think that we need to:
>
> - #undef stat before including bfd headers.
> - Redefine it afterwards back to rpl_stat (*)
> - add some kind of wrapper around bfd_stat (like maybe called gdb_bfd_stat)

Wouldn't it be easier to #define GNULIB_NAMESPACE in this file? That
way, the gnulib stuff stays in gnulib:: (or gdb::, or whatever), and
the global ::stat is the system one.

>   that handles the mismatch.  Something like:
>
> int
> gdb_bfd_stat (bfd *abfd, struct stat *statbuf)
> {
> #undef stat
>   struct stat st;
>
>   int res = bfd_stat (abfd, &st);
>   if (res != 0)
>     return res;
>
> #define COPY(FIELD) statbuf->FIELD = st.FIELD
>   COPY (st_dev);
>   // ... copy over all relevant fields ...
> #undef COPY
>
> #define stat rpl_stat
> }
>
> (*) there's probably some '#ifdef _GL...' we can check to know
> whether we need to do this.
>
> Thanks,
> Pedro Alves
>