* [PATCH v2 2/3] gdb: Rename fputs_unfiltered to ui_file_puts.
@ 2019-11-29 0:03 Iain Buclaw
[not found] ` <lx0Egw9mNmlYkLhQI82kbTPPFKKjliRfIS1ROwNvoxDvNuZHP6AUrjS-Y2S5hXch7J8Jv8BIsJJQP60jFk3rrg==@protonmail.internalid>
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Iain Buclaw @ 2019-11-29 0:03 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves
[-- Attachment #1: Type: text/plain, Size: 994 bytes --]
This patch redefines fputs_unfiltered in utils.c, with new behavior to forward parameters to fputs_maybe_filtered. This makes fputs_unfiltered identical to fputs_filtered, except filtering is disabled.
Some callers of fputs_unfiltered have been updated to use ui_file_puts where they were using other ui_file_* functions anyway for IO.
This fixes the problem I saw with \032\032post-prompt annotation being flushed to stdout in the wrong order.
--
Iain
---
gdb/ChangeLog:
2019-11-29 Iain Buclaw <ibuclaw@gdcproject.org>
* gdb/remote-sim.c (gdb_os_write_stderr): Update.
* gdb/remote.c (remote_console_output): Update.
* gdb/ui-file.c (fputs_unfiltered): Rename to...
(ui_file_puts): ...this.
* gdb/ui-file.h (ui_file_puts): Add declaration.
* gdb/utils.c (emit_style_escape): Update.
(flush_wrap_buffer): Update.
(fputs_maybe_filtered): Update.
(fputs_unfiltered): Add function.
---
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: uifileputs.patch --]
[-- Type: text/x-patch; name="uifileputs.patch", Size: 3316 bytes --]
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index f9c2f605c3..b7eeccf91b 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -378,7 +378,7 @@ gdb_os_write_stderr (host_callback *p, const char *buf, int len)
{
b[0] = buf[i];
b[1] = 0;
- fputs_unfiltered (b, gdb_stdtargerr);
+ ui_file_puts (gdb_stdtargerr, b);
}
return len;
}
diff --git a/gdb/remote.c b/gdb/remote.c
index 054802f744..7cc2a0470b 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -6782,7 +6782,7 @@ remote_console_output (const char *msg)
tb[0] = c;
tb[1] = 0;
- fputs_unfiltered (tb, gdb_stdtarg);
+ ui_file_puts (gdb_stdtarg, tb);
}
ui_file_flush (gdb_stdtarg);
}
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index d7d113856e..6a3ebfb6a9 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -150,7 +150,7 @@ ui_file_read (struct ui_file *file, char *buf, long length_buf)
}
void
-fputs_unfiltered (const char *buf, struct ui_file *file)
+ui_file_puts (struct ui_file *file, const char *buf)
{
file->puts (buf);
}
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index 711a888a2e..8b57622a09 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -112,6 +112,8 @@ extern void ui_file_write_async_safe (struct ui_file *file, const char *buf,
extern long ui_file_read (struct ui_file *file, char *buf, long length_buf);
+extern void ui_file_puts (struct ui_file *file, const char *buf);
+
extern int gdb_console_fputs (const char *, FILE *);
/* A std::string-based ui_file. Can be used as a scratch buffer for
diff --git a/gdb/utils.c b/gdb/utils.c
index 5d6f680bce..0e09f646bf 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1405,7 +1405,7 @@ emit_style_escape (const ui_file_style &style,
if (stream == nullptr)
wrap_buffer.append (style.to_ansi ());
else
- fputs_unfiltered (style.to_ansi ().c_str (), stream);
+ ui_file_puts (stream, style.to_ansi ().c_str ());
}
/* Set the current output style. This will affect future uses of the
@@ -1539,7 +1539,7 @@ flush_wrap_buffer (struct ui_file *stream)
{
if (stream == gdb_stdout && !wrap_buffer.empty ())
{
- fputs_unfiltered (wrap_buffer.c_str (), stream);
+ ui_file_puts (stream, wrap_buffer.c_str ());
wrap_buffer.clear ();
}
}
@@ -1695,7 +1695,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
|| top_level_interpreter ()->interp_ui_out ()->is_mi_like_p ())
{
flush_wrap_buffer (stream);
- fputs_unfiltered (linebuffer, stream);
+ ui_file_puts (stream, linebuffer);
return;
}
@@ -1795,7 +1795,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
/* Now output indentation and wrapped string. */
if (wrap_column)
{
- fputs_unfiltered (wrap_indent, stream);
+ ui_file_puts (stream, wrap_indent);
if (stream->can_emit_style_escape ())
emit_style_escape (save_style, stream);
/* FIXME, this strlen is what prevents wrap_indent from
@@ -1833,6 +1833,12 @@ fputs_filtered (const char *linebuffer, struct ui_file *stream)
fputs_maybe_filtered (linebuffer, stream, 1);
}
+void
+fputs_unfiltered (const char *linebuffer, struct ui_file *stream)
+{
+ fputs_maybe_filtered (linebuffer, stream, 0);
+}
+
/* See utils.h. */
void
^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <lx0Egw9mNmlYkLhQI82kbTPPFKKjliRfIS1ROwNvoxDvNuZHP6AUrjS-Y2S5hXch7J8Jv8BIsJJQP60jFk3rrg==@protonmail.internalid>]
* Re: [PATCH v2 2/3] gdb: Rename fputs_unfiltered to ui_file_puts.
2019-11-29 0:03 [PATCH v2 2/3] gdb: Rename fputs_unfiltered to ui_file_puts Iain Buclaw
[not found] ` <lx0Egw9mNmlYkLhQI82kbTPPFKKjliRfIS1ROwNvoxDvNuZHP6AUrjS-Y2S5hXch7J8Jv8BIsJJQP60jFk3rrg==@protonmail.internalid>
@ 2019-12-01 15:12 ` Simon Marchi
2019-12-01 15:15 ` Simon Marchi
2020-01-26 11:40 ` Joel Brobecker
2 siblings, 1 reply; 5+ messages in thread
From: Simon Marchi @ 2019-12-01 15:12 UTC (permalink / raw)
To: Iain Buclaw, gdb-patches; +Cc: Pedro Alves
On 2019-11-28 7:03 p.m., Iain Buclaw wrote:
> This patch redefines fputs_unfiltered in utils.c, with new behavior to forward parameters to fputs_maybe_filtered. This makes fputs_unfiltered identical to fputs_filtered, except filtering is disabled.
>
> Some callers of fputs_unfiltered have been updated to use ui_file_puts where they were using other ui_file_* functions anyway for IO.
>
> This fixes the problem I saw with \032\032post-prompt annotation being flushed to stdout in the wrong order.
>
> --
> Iain
>
> ---
> gdb/ChangeLog:
>
> 2019-11-29 Iain Buclaw <ibuclaw@gdcproject.org>
>
> * gdb/remote-sim.c (gdb_os_write_stderr): Update.
> * gdb/remote.c (remote_console_output): Update.
> * gdb/ui-file.c (fputs_unfiltered): Rename to...
> (ui_file_puts): ...this.
> * gdb/ui-file.h (ui_file_puts): Add declaration.
> * gdb/utils.c (emit_style_escape): Update.
> (flush_wrap_buffer): Update.
> (fputs_maybe_filtered): Update.
> (fputs_unfiltered): Add function.
>
> ---
>
>
Hi Iain,
The patch does not apply for me on current master:
$ git apply ~/patches/uifileputs.patch
error: patch failed: gdb/remote.c:6782
error: gdb/remote.c: patch does not apply
How did you generate this patch? Could you please send your patches using git-send-email?
Simon
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/3] gdb: Rename fputs_unfiltered to ui_file_puts.
2019-12-01 15:12 ` Simon Marchi
@ 2019-12-01 15:15 ` Simon Marchi
0 siblings, 0 replies; 5+ messages in thread
From: Simon Marchi @ 2019-12-01 15:15 UTC (permalink / raw)
To: Iain Buclaw, gdb-patches; +Cc: Pedro Alves
On 2019-12-01 10:12 a.m., Simon Marchi wrote:
> On 2019-11-28 7:03 p.m., Iain Buclaw wrote:
>> This patch redefines fputs_unfiltered in utils.c, with new behavior to forward parameters to fputs_maybe_filtered. This makes fputs_unfiltered identical to fputs_filtered, except filtering is disabled.
>>
>> Some callers of fputs_unfiltered have been updated to use ui_file_puts where they were using other ui_file_* functions anyway for IO.
>>
>> This fixes the problem I saw with \032\032post-prompt annotation being flushed to stdout in the wrong order.
>>
>> --
>> Iain
>>
>> ---
>> gdb/ChangeLog:
>>
>> 2019-11-29 Iain Buclaw <ibuclaw@gdcproject.org>
>>
>> * gdb/remote-sim.c (gdb_os_write_stderr): Update.
>> * gdb/remote.c (remote_console_output): Update.
>> * gdb/ui-file.c (fputs_unfiltered): Rename to...
>> (ui_file_puts): ...this.
>> * gdb/ui-file.h (ui_file_puts): Add declaration.
>> * gdb/utils.c (emit_style_escape): Update.
>> (flush_wrap_buffer): Update.
>> (fputs_maybe_filtered): Update.
>> (fputs_unfiltered): Add function.
>>
>> ---
>>
>>
>
> Hi Iain,
>
> The patch does not apply for me on current master:
>
> $ git apply ~/patches/uifileputs.patch
> error: patch failed: gdb/remote.c:6782
> error: gdb/remote.c: patch does not apply
>
> How did you generate this patch? Could you please send your patches using git-send-email?
>
> Simon
My bad, I had not seen that this was part of a series (the email is not threaded with the others
of the series). After applying 1/3, this one applies fine.
Simon
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/3] gdb: Rename fputs_unfiltered to ui_file_puts.
2019-11-29 0:03 [PATCH v2 2/3] gdb: Rename fputs_unfiltered to ui_file_puts Iain Buclaw
[not found] ` <lx0Egw9mNmlYkLhQI82kbTPPFKKjliRfIS1ROwNvoxDvNuZHP6AUrjS-Y2S5hXch7J8Jv8BIsJJQP60jFk3rrg==@protonmail.internalid>
2019-12-01 15:12 ` Simon Marchi
@ 2020-01-26 11:40 ` Joel Brobecker
2020-08-07 22:15 ` Iain Buclaw
2 siblings, 1 reply; 5+ messages in thread
From: Joel Brobecker @ 2020-01-26 11:40 UTC (permalink / raw)
To: Iain Buclaw; +Cc: gdb-patches, Pedro Alves
Hi Iain,
> This patch redefines fputs_unfiltered in utils.c, with new behavior to forward parameters to fputs_maybe_filtered. This makes fputs_unfiltered identical to fputs_filtered, except filtering is disabled.
>
> Some callers of fputs_unfiltered have been updated to use ui_file_puts where they were using other ui_file_* functions anyway for IO.
>
> This fixes the problem I saw with \032\032post-prompt annotation being flushed to stdout in the wrong order.
>
> --
> Iain
>
> ---
> gdb/ChangeLog:
>
> 2019-11-29 Iain Buclaw <ibuclaw@gdcproject.org>
>
> * gdb/remote-sim.c (gdb_os_write_stderr): Update.
> * gdb/remote.c (remote_console_output): Update.
> * gdb/ui-file.c (fputs_unfiltered): Rename to...
> (ui_file_puts): ...this.
> * gdb/ui-file.h (ui_file_puts): Add declaration.
> * gdb/utils.c (emit_style_escape): Update.
> (flush_wrap_buffer): Update.
> (fputs_maybe_filtered): Update.
> (fputs_unfiltered): Add function.
As far as I can tell, no one has reviewed this patch. This is a call
for help from other Global Maintainers, as I don't feel qualified
to review it all on my own.
As far as I can tell from reading this patch, it really feels like
you are trying to turn the turn the relationship of filtered vs
unfiltered on its head. Right now, fputs_maybe_filtered is the one
that depends on fputs_unfiltered, and this is realy what feels
natural to me.
For some reason, you decided to do the other way around, but without
a real explanation of why we are getting text being printed in the wrong
order, and why your patch fixes it or why you think this is the best
approach.
>
> ---
>
>
> diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
> index f9c2f605c3..b7eeccf91b 100644
> --- a/gdb/remote-sim.c
> +++ b/gdb/remote-sim.c
> @@ -378,7 +378,7 @@ gdb_os_write_stderr (host_callback *p, const char *buf, int len)
> {
> b[0] = buf[i];
> b[1] = 0;
> - fputs_unfiltered (b, gdb_stdtargerr);
> + ui_file_puts (gdb_stdtargerr, b);
> }
> return len;
> }
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 054802f744..7cc2a0470b 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -6782,7 +6782,7 @@ remote_console_output (const char *msg)
>
> tb[0] = c;
> tb[1] = 0;
> - fputs_unfiltered (tb, gdb_stdtarg);
> + ui_file_puts (gdb_stdtarg, tb);
> }
> ui_file_flush (gdb_stdtarg);
> }
> diff --git a/gdb/ui-file.c b/gdb/ui-file.c
> index d7d113856e..6a3ebfb6a9 100644
> --- a/gdb/ui-file.c
> +++ b/gdb/ui-file.c
> @@ -150,7 +150,7 @@ ui_file_read (struct ui_file *file, char *buf, long length_buf)
> }
>
> void
> -fputs_unfiltered (const char *buf, struct ui_file *file)
> +ui_file_puts (struct ui_file *file, const char *buf)
> {
> file->puts (buf);
> }
> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
> index 711a888a2e..8b57622a09 100644
> --- a/gdb/ui-file.h
> +++ b/gdb/ui-file.h
> @@ -112,6 +112,8 @@ extern void ui_file_write_async_safe (struct ui_file *file, const char *buf,
>
> extern long ui_file_read (struct ui_file *file, char *buf, long length_buf);
>
> +extern void ui_file_puts (struct ui_file *file, const char *buf);
> +
> extern int gdb_console_fputs (const char *, FILE *);
>
> /* A std::string-based ui_file. Can be used as a scratch buffer for
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 5d6f680bce..0e09f646bf 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -1405,7 +1405,7 @@ emit_style_escape (const ui_file_style &style,
> if (stream == nullptr)
> wrap_buffer.append (style.to_ansi ());
> else
> - fputs_unfiltered (style.to_ansi ().c_str (), stream);
> + ui_file_puts (stream, style.to_ansi ().c_str ());
> }
>
> /* Set the current output style. This will affect future uses of the
> @@ -1539,7 +1539,7 @@ flush_wrap_buffer (struct ui_file *stream)
> {
> if (stream == gdb_stdout && !wrap_buffer.empty ())
> {
> - fputs_unfiltered (wrap_buffer.c_str (), stream);
> + ui_file_puts (stream, wrap_buffer.c_str ());
> wrap_buffer.clear ();
> }
> }
> @@ -1695,7 +1695,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
> || top_level_interpreter ()->interp_ui_out ()->is_mi_like_p ())
> {
> flush_wrap_buffer (stream);
> - fputs_unfiltered (linebuffer, stream);
> + ui_file_puts (stream, linebuffer);
> return;
> }
>
> @@ -1795,7 +1795,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
> /* Now output indentation and wrapped string. */
> if (wrap_column)
> {
> - fputs_unfiltered (wrap_indent, stream);
> + ui_file_puts (stream, wrap_indent);
> if (stream->can_emit_style_escape ())
> emit_style_escape (save_style, stream);
> /* FIXME, this strlen is what prevents wrap_indent from
> @@ -1833,6 +1833,12 @@ fputs_filtered (const char *linebuffer, struct ui_file *stream)
> fputs_maybe_filtered (linebuffer, stream, 1);
> }
>
> +void
> +fputs_unfiltered (const char *linebuffer, struct ui_file *stream)
> +{
> + fputs_maybe_filtered (linebuffer, stream, 0);
> +}
> +
> /* See utils.h. */
>
> void
--
Joel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/3] gdb: Rename fputs_unfiltered to ui_file_puts.
2020-01-26 11:40 ` Joel Brobecker
@ 2020-08-07 22:15 ` Iain Buclaw
0 siblings, 0 replies; 5+ messages in thread
From: Iain Buclaw @ 2020-08-07 22:15 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches, Pedro Alves
Excerpts from Joel Brobecker's message of January 26, 2020 12:31 pm:
> Hi Iain,
>
>> This patch redefines fputs_unfiltered in utils.c, with new behavior to forward parameters to fputs_maybe_filtered. This makes fputs_unfiltered identical to fputs_filtered, except filtering is disabled.
>>
>> Some callers of fputs_unfiltered have been updated to use ui_file_puts where they were using other ui_file_* functions anyway for IO.
>>
>> This fixes the problem I saw with \032\032post-prompt annotation being flushed to stdout in the wrong order.
>>
>> --
>> Iain
>>
>> ---
>> gdb/ChangeLog:
>>
>> 2019-11-29 Iain Buclaw <ibuclaw@gdcproject.org>
>>
>> * gdb/remote-sim.c (gdb_os_write_stderr): Update.
>> * gdb/remote.c (remote_console_output): Update.
>> * gdb/ui-file.c (fputs_unfiltered): Rename to...
>> (ui_file_puts): ...this.
>> * gdb/ui-file.h (ui_file_puts): Add declaration.
>> * gdb/utils.c (emit_style_escape): Update.
>> (flush_wrap_buffer): Update.
>> (fputs_maybe_filtered): Update.
>> (fputs_unfiltered): Add function.
>
> As far as I can tell, no one has reviewed this patch. This is a call
> for help from other Global Maintainers, as I don't feel qualified
> to review it all on my own.
>
> As far as I can tell from reading this patch, it really feels like
> you are trying to turn the turn the relationship of filtered vs
> unfiltered on its head. Right now, fputs_maybe_filtered is the one
> that depends on fputs_unfiltered, and this is realy what feels
> natural to me.
>
> For some reason, you decided to do the other way around, but without
> a real explanation of why we are getting text being printed in the wrong
> order, and why your patch fixes it or why you think this is the best
> approach.
>
>
Hi Joel,
I guess that someone answered your question, as it appears that this
patch got committed anyway.
To give my best explanation on what I saw anyway. There are a number of
print related functions that each followed a consistent behaviour except
fputs_unfiltered.
- printf_filtered called vfprintf_maybe_filtered(/*filter=*/true).
- printf_unfiltered called vfprintf_maybe_filtered(false).
- fprintf_filtered called vfprintf_maybe_filtered(true).
- fprintf_unfiltered called vfprintf_maybe_filtered(false).
- fputs_styled called fputs_maybe_filtered(/*filter=*/1).
- fputs_styled_unfiltered called fputs_maybe_filtered(0).
- fputs_filtered called fputs_maybe_filtered(1).
- fputs_unfiltered called ui_file::puts()
Before 2a3c1174c3c0db1140180fb3fc56ac324d1c0a7c, all *printf_unfiltered
functions used to call fputs_unfiltered(), so there was no problems
mixing printf_unfiltered() and fputs_unfiltered().
After the change though, printf_unfiltered() now got written to a buffer
awaiting to be flushed whilst fputs_unfiltered() continued to write
directly to the output file.
This is the part that caused the regression:
---
@@ -2064,13 +2096,13 @@ vfprintf_unfiltered (struct ui_file *stream, const char *format, va_list args)
fputs_unfiltered (timestamp.c_str (), stream);
}
else
- fputs_unfiltered (linebuffer.c_str (), stream);
+ vfprintf_maybe_filtered (stream, format, args, false, true);
}
void
vprintf_filtered (const char *format, va_list args)
---
Reverting the one line caused other problems however, so the only way I
saw to reasonably move forward was to bring fputs_unfiltered in line
with all others I've listed above, so now it calls fputs_maybe_filtered(0),
and no one has to worry about printf_unfiltered and fputs_unfiltered
being flushed in a different order to the one they were called in.
Hope this is clear.
Iain.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-07 22:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-29 0:03 [PATCH v2 2/3] gdb: Rename fputs_unfiltered to ui_file_puts Iain Buclaw
[not found] ` <lx0Egw9mNmlYkLhQI82kbTPPFKKjliRfIS1ROwNvoxDvNuZHP6AUrjS-Y2S5hXch7J8Jv8BIsJJQP60jFk3rrg==@protonmail.internalid>
2019-12-01 15:12 ` Simon Marchi
2019-12-01 15:15 ` Simon Marchi
2020-01-26 11:40 ` Joel Brobecker
2020-08-07 22:15 ` Iain Buclaw
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).