[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160918203608.GB15995@brightrain.aerifal.cx>
Date: Sun, 18 Sep 2016 16:36:08 -0400
From: Rich Felker <dalias@...c.org>
To: musl <musl@...ts.openwall.com>
Subject: Re: [PATCH] make fflush_unlocked(NULL) work
On Sun, Sep 18, 2016 at 09:44:55PM +0200, Denys Vlasenko wrote:
> In glibc, fflush_unlocked(NULL) works.
> Before this patch, musl was segfaulting.
>
> Signed-off-by: Denys Vlasenko <vda.linux@...glemail.com>
> CC: musl <musl@...ts.openwall.com>
> ---
> src/stdio/fflush.c | 35 +++++++++++++++++++++++++----------
> 1 file changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/src/stdio/fflush.c b/src/stdio/fflush.c
> index 3f462c8..06d7a56 100644
> --- a/src/stdio/fflush.c
> +++ b/src/stdio/fflush.c
> @@ -2,20 +2,35 @@
>
> static int __fflush_unlocked(FILE *f)
> {
> - /* If writing, flush output */
> - if (f->wpos > f->wbase) {
> - f->write(f, 0, 0);
> - if (!f->wpos) return EOF;
> + int r;
> +
> + if (f) {
> + /* If writing, flush output */
> + if (f->wpos > f->wbase) {
> + f->write(f, 0, 0);
> + if (!f->wpos) return EOF;
> + }
> +
> + /* If reading, sync position, per POSIX */
> + if (f->rpos < f->rend) f->seek(f, f->rpos-f->rend, SEEK_CUR);
> +
> + /* Clear read and write modes */
> + f->wpos = f->wbase = f->wend = 0;
> + f->rpos = f->rend = 0;
> +
> + return 0;
> }
>
> - /* If reading, sync position, per POSIX */
> - if (f->rpos < f->rend) f->seek(f, f->rpos-f->rend, SEEK_CUR);
> + /* fflush_unlocked(NULL) is supported by glibc, mimic that */
>
> - /* Clear read and write modes */
> - f->wpos = f->wbase = f->wend = 0;
> - f->rpos = f->rend = 0;
> + r = __stdout_used ? __fflush_unlocked(__stdout_used) : 0;
>
> - return 0;
> + for (f=*__ofl_lock(); f; f=f->next) {
> + if (f->wpos > f->wbase) r |= __fflush_unlocked(f);
> + }
> + __ofl_unlock();
> +
> + return r;
> }
>
> /* stdout.c will override this if linked */
This patch introduces significant code duplication and complexity for
the sake of saving something like 10 cycles in an operation that makes
syscalls (i.e. takes thousands if not tens of thousands of cycles). As
mentioned on the bb list, the right fix is just making fflush_unlocked
an alias for fflush. Then __fflush_unlocked can be eliminated
completely and the file simplified rather than increased in
complexity. See the attached patch which I'll apply if there are no
obvious mistakes or objections.
Rich
View attachment "fflush_unlocked.diff" of type "text/plain" (1473 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
