| Message ID | E1fVCWO-0004eX-G7@mid.deneb.enyo.de |
|---|---|
| State | New |
| Headers | show |
| Series | Assume that _IO_MTSAFE_IO is always defined in libio/stdfiles.c | expand |
On Jun 19 2018, Florian Weimer <fw@deneb.enyo.de> wrote: > * libio/stdfiles.c (DEF_STDFILE): Assume that _IO_MTSAFE_IO is > always defined. Why only here? Andreas.
* Andreas Schwab: > On Jun 19 2018, Florian Weimer <fw@deneb.enyo.de> wrote: > >> * libio/stdfiles.c (DEF_STDFILE): Assume that _IO_MTSAFE_IO is >> always defined. > > Why only here? _IO_MTSAFE_IO has a bit of a checkered history. Previous attempts at removal failed when they encountered resistance in the form of parts of the library that were *not* compiled with _IO_MTSAFE_IO. I need to change DEF_STDFILE to fix bug 23313.
On Jun 19 2018, Florian Weimer <fw@deneb.enyo.de> wrote: > * Andreas Schwab: > >> On Jun 19 2018, Florian Weimer <fw@deneb.enyo.de> wrote: >> >>> * libio/stdfiles.c (DEF_STDFILE): Assume that _IO_MTSAFE_IO is >>> always defined. >> >> Why only here? > > _IO_MTSAFE_IO has a bit of a checkered history. Previous attempts at > removal failed when they encountered resistance in the form of parts > of the library that were *not* compiled with _IO_MTSAFE_IO. > > I need to change DEF_STDFILE to fix bug 23313. That bug isn't even referenced here. Andreas.
* Andreas Schwab: > On Jun 19 2018, Florian Weimer <fw@deneb.enyo.de> wrote: > >> * Andreas Schwab: >> >>> On Jun 19 2018, Florian Weimer <fw@deneb.enyo.de> wrote: >>> >>>> * libio/stdfiles.c (DEF_STDFILE): Assume that _IO_MTSAFE_IO is >>>> always defined. >>> >>> Why only here? >> >> _IO_MTSAFE_IO has a bit of a checkered history. Previous attempts at >> removal failed when they encountered resistance in the form of parts >> of the library that were *not* compiled with _IO_MTSAFE_IO. >> >> I need to change DEF_STDFILE to fix bug 23313. > > That bug isn't even referenced here. Yes, it's an independent change. Is the patch okay? I can rework the upcoming patch to change the dead code, but I'd prefer the cleanup.
On Jun 19 2018, Florian Weimer <fw@deneb.enyo.de> wrote: > * Andreas Schwab: > >> On Jun 19 2018, Florian Weimer <fw@deneb.enyo.de> wrote: >> >>> * Andreas Schwab: >>> >>>> On Jun 19 2018, Florian Weimer <fw@deneb.enyo.de> wrote: >>>> >>>>> * libio/stdfiles.c (DEF_STDFILE): Assume that _IO_MTSAFE_IO is >>>>> always defined. >>>> >>>> Why only here? >>> >>> _IO_MTSAFE_IO has a bit of a checkered history. Previous attempts at >>> removal failed when they encountered resistance in the form of parts >>> of the library that were *not* compiled with _IO_MTSAFE_IO. >>> >>> I need to change DEF_STDFILE to fix bug 23313. >> >> That bug isn't even referenced here. > > Yes, it's an independent change. I don't understand. Either it fixes a bug or it is pointless. Andreas.
* Andreas Schwab: > On Jun 19 2018, Florian Weimer <fw@deneb.enyo.de> wrote: > >> * Andreas Schwab: >> >>> On Jun 19 2018, Florian Weimer <fw@deneb.enyo.de> wrote: >>> >>>> * Andreas Schwab: >>>> >>>>> On Jun 19 2018, Florian Weimer <fw@deneb.enyo.de> wrote: >>>>> >>>>>> * libio/stdfiles.c (DEF_STDFILE): Assume that _IO_MTSAFE_IO is >>>>>> always defined. >>>>> >>>>> Why only here? >>>> >>>> _IO_MTSAFE_IO has a bit of a checkered history. Previous attempts at >>>> removal failed when they encountered resistance in the form of parts >>>> of the library that were *not* compiled with _IO_MTSAFE_IO. >>>> >>>> I need to change DEF_STDFILE to fix bug 23313. >>> >>> That bug isn't even referenced here. >> >> Yes, it's an independent change. > > I don't understand. Either it fixes a bug or it is pointless. Please look at the other patch: <https://sourceware.org/ml/libc-alpha/2018-06/msg00543.html>
On Jun 19 2018, Florian Weimer <fw@deneb.enyo.de> wrote: > Please look at the other patch: > > <https://sourceware.org/ml/libc-alpha/2018-06/msg00543.html> And what do I see there? Andreas.
* Andreas Schwab: > On Jun 19 2018, Florian Weimer <fw@deneb.enyo.de> wrote: > >> Please look at the other patch: >> >> <https://sourceware.org/ml/libc-alpha/2018-06/msg00543.html> > > And what do I see there? The first hunk changes the definition of DEF_STDFILE: | diff --git a/libio/stdfiles.c b/libio/stdfiles.c | index 18e1172ad0..2435f412f2 100644 | --- a/libio/stdfiles.c | +++ b/libio/stdfiles.c | @@ -33,11 +33,19 @@ | | #include "libioP.h" | | +#ifdef SHARED | +/* Place the variables defined below in a separate section for the | + interposition check in vtables.c. */ | +# define STDFILE_SECTION __attribute__ ((section ("__libc_IO_stdfiles"))) | +#else | +# define STDFILE_SECTION | +#endif | + | #define DEF_STDFILE(NAME, FD, CHAIN, FLAGS) \ | static _IO_lock_t _IO_stdfile_##FD##_lock = _IO_lock_initializer; \ | static struct _IO_wide_data _IO_wide_data_##FD \ | = { ._wide_vtable = &_IO_wfile_jumps }; \ | - struct _IO_FILE_plus NAME \ | + struct _IO_FILE_plus NAME STDFILE_SECTION \ | = {FILEBUF_LITERAL(CHAIN, FLAGS, FD, &_IO_wide_data_##FD), \ | &_IO_file_jumps}; Without the cleanup, the change would have to happen in two places.
On Jun 19 2018, Florian Weimer <fw@deneb.enyo.de> wrote:
> Without the cleanup, the change would have to happen in two places.
And what's wrong with that?
Andreas.
diff --git a/libio/stdfiles.c b/libio/stdfiles.c index 8d96f0b65c..18e1172ad0 100644 --- a/libio/stdfiles.c +++ b/libio/stdfiles.c @@ -33,22 +33,13 @@ #include "libioP.h" -#ifdef _IO_MTSAFE_IO -# define DEF_STDFILE(NAME, FD, CHAIN, FLAGS) \ +#define DEF_STDFILE(NAME, FD, CHAIN, FLAGS) \ static _IO_lock_t _IO_stdfile_##FD##_lock = _IO_lock_initializer; \ static struct _IO_wide_data _IO_wide_data_##FD \ = { ._wide_vtable = &_IO_wfile_jumps }; \ struct _IO_FILE_plus NAME \ = {FILEBUF_LITERAL(CHAIN, FLAGS, FD, &_IO_wide_data_##FD), \ &_IO_file_jumps}; -#else -# define DEF_STDFILE(NAME, FD, CHAIN, FLAGS) \ - static struct _IO_wide_data _IO_wide_data_##FD \ - = { ._wide_vtable = &_IO_wfile_jumps }; \ - struct _IO_FILE_plus NAME \ - = {FILEBUF_LITERAL(CHAIN, FLAGS, FD, &_IO_wide_data_##FD), \ - &_IO_file_jumps}; -#endif DEF_STDFILE(_IO_2_1_stdin_, 0, 0, _IO_NO_WRITES); DEF_STDFILE(_IO_2_1_stdout_, 1, &_IO_2_1_stdin_, _IO_NO_READS);