[PATCH] Consistently Use ui_file parameter to show callbacks

Joel Brobecker brobecker@adacore.com
Wed Dec 29 04:05:56 GMT 2021


Hi Tom,

On Tue, Dec 28, 2021 at 02:44:24PM -0700, Tom Tromey wrote:
> I happened to notice that one "show" callback was printing to
> gdb_stdout rather than to the passed-in ui_file parameter.  I went
> through all such callbacks and fixed them to consistently use the
> ui_file, and furthermore to use filtered output.

Thanks for this nice little consistency improvement!

One minor comment is the fact that this commit is combining
the consistent use of the passed-in ui_file, and the use of
filtered output. I prefer to avoid those myself, but what
made me raise this in this case is the fact that the commit's
subject makes it sound like the only only change is the consitent
use of the passed-in ui_file.

For that, my preferred suggestion would be to split the fix
towards filtered output to a second patch. But I would be fine
with a subject slightly reworked to mention both changes.

Other than that, the change looks good to me. I thought I had
noticed a couple of spots where the parameter alignment needed
fixing, but in double-checking once I removed the +/-, they
were OK (ah, tab characters...)

> 
> Regression tested on x86-64 Fedora 34.
> ---
>  gdb/arch-utils.c          | 16 ++++++++--------
>  gdb/arm-tdep.c            |  2 +-
>  gdb/cli/cli-logging.c     |  4 ++--
>  gdb/debuginfod-support.c  |  7 ++++---
>  gdb/exec.c                |  2 +-
>  gdb/infcmd.c              |  6 +++---
>  gdb/language.c            | 14 +++++++-------
>  gdb/mips-tdep.c           |  9 +++++----
>  gdb/record-btrace.c       |  2 +-
>  gdb/remote.c              | 20 ++++++++++----------
>  gdb/source.c              | 12 ++++++------
>  gdb/target-descriptions.c | 10 ++++++----
>  gdb/tui/tui-win.c         |  4 ++--
>  13 files changed, 56 insertions(+), 52 deletions(-)
> 
> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
> index 4442ec9b82f..e67fbc2cbd4 100644
> --- a/gdb/arch-utils.c
> +++ b/gdb/arch-utils.c
> @@ -375,18 +375,18 @@ show_endian (struct ui_file *file, int from_tty, struct cmd_list_element *c,
>  {
>    if (target_byte_order_user == BFD_ENDIAN_UNKNOWN)
>      if (gdbarch_byte_order (get_current_arch ()) == BFD_ENDIAN_BIG)
> -      fprintf_unfiltered (file, _("The target endianness is set automatically "
> -				  "(currently big endian).\n"));
> +      fprintf_filtered (file, _("The target endianness is set automatically "
> +				"(currently big endian).\n"));
>      else
> -      fprintf_unfiltered (file, _("The target endianness is set automatically "
> -				  "(currently little endian).\n"));
> +      fprintf_filtered (file, _("The target endianness is set automatically "
> +				"(currently little endian).\n"));
>    else
>      if (target_byte_order_user == BFD_ENDIAN_BIG)
> -      fprintf_unfiltered (file,
> -			  _("The target is set to big endian.\n"));
> +      fprintf_filtered (file,
> +			_("The target is set to big endian.\n"));
>      else
> -      fprintf_unfiltered (file,
> -			  _("The target is set to little endian.\n"));
> +      fprintf_filtered (file,
> +			_("The target is set to little endian.\n"));
>  }
>  
>  static void
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index e0ef1d2946e..f5691f02e21 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -8623,7 +8623,7 @@ show_disassembly_style_sfunc (struct ui_file *file, int from_tty,
>  	len = strcspn (style, ",");
>        }
>  
> -  fprintf_unfiltered (file, "The disassembly style is \"%.*s\".\n", len, style);
> +  fprintf_filtered (file, "The disassembly style is \"%.*s\".\n", len, style);
>  }
>  
>  /* Return the ARM register name corresponding to register I.  */
> diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c
> index 124d15c60cf..9afd51e352a 100644
> --- a/gdb/cli/cli-logging.c
> +++ b/gdb/cli/cli-logging.c
> @@ -181,9 +181,9 @@ show_logging_enabled (struct ui_file *file, int from_tty,
>  		       struct cmd_list_element *c, const char *value)
>  {
>    if (logging_enabled)
> -    printf_unfiltered (_("Logging is enabled.\n"));
> +    fprintf_filtered (file, _("Logging is enabled.\n"));
>    else
> -    printf_unfiltered (_("Logging is disabled.\n"));
> +    fprintf_filtered (file, _("Logging is disabled.\n"));
>  }
>  
>  void _initialize_cli_logging ();
> diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
> index 1f160e29714..cbdd992bcf3 100644
> --- a/gdb/debuginfod-support.c
> +++ b/gdb/debuginfod-support.c
> @@ -299,8 +299,9 @@ static void
>  show_debuginfod_enabled (ui_file *file, int from_tty, cmd_list_element *cmd,
>  			 const char *value)
>  {
> -  printf_unfiltered (_("Debuginfod functionality is currently set to "
> -		       "\"%s\".\n"), debuginfod_enabled);
> +  fprintf_unfiltered (file,
> +		      _("Debuginfod functionality is currently set to "
> +			"\"%s\".\n"), debuginfod_enabled);
>  }
>  
>  /* Set callback for "set debuginfod urls".  */
> @@ -341,7 +342,7 @@ show_debuginfod_urls (ui_file *file, int from_tty, cmd_list_element *cmd,
>  		      const char *value)
>  {
>    if (value[0] == '\0')
> -    fprintf_unfiltered (file, _("Debuginfod URLs have not been set.\n"));
> +    fprintf_filtered (file, _("Debuginfod URLs have not been set.\n"));
>    else
>      fprintf_filtered (file, _("Debuginfod URLs are currently set to:\n%s\n"),
>  		      value);
> diff --git a/gdb/exec.c b/gdb/exec.c
> index 1be51474b4a..3d4cbdfcf45 100644
> --- a/gdb/exec.c
> +++ b/gdb/exec.c
> @@ -102,7 +102,7 @@ static void
>  show_exec_file_mismatch_command (struct ui_file *file, int from_tty,
>  				 struct cmd_list_element *c, const char *value)
>  {
> -  fprintf_filtered (gdb_stdout,
> +  fprintf_filtered (file,
>  		    _("exec-file-mismatch handling is currently \"%s\".\n"),
>  		    exec_file_mismatch_names[exec_file_mismatch_mode]);
>  }
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index a3752cc9285..d00a70c4361 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -117,7 +117,7 @@ show_inferior_tty_command (struct ui_file *file, int from_tty,
>       directly.  */
>    const std::string &inferior_tty = current_inferior ()->tty ();
>  
> -  fprintf_filtered (gdb_stdout,
> +  fprintf_filtered (file,
>  		    _("Terminal for future runs of program being debugged "
>  		      "is \"%s\".\n"), inferior_tty.c_str ());
>  }
> @@ -177,13 +177,13 @@ show_cwd_command (struct ui_file *file, int from_tty,
>    const std::string &cwd = current_inferior ()->cwd ();
>  
>    if (cwd.empty ())
> -    fprintf_filtered (gdb_stdout,
> +    fprintf_filtered (file,
>  		      _("\
>  You have not set the inferior's current working directory.\n\
>  The inferior will inherit GDB's cwd if native debugging, or the remote\n\
>  server's cwd if remote debugging.\n"));
>    else
> -    fprintf_filtered (gdb_stdout,
> +    fprintf_filtered (file,
>  		      _("Current working directory that will be used "
>  			"when starting the inferior is \"%s\".\n"),
>  		      cwd.c_str ());
> diff --git a/gdb/language.c b/gdb/language.c
> index 3e60a668ed6..c4068ec446e 100644
> --- a/gdb/language.c
> +++ b/gdb/language.c
> @@ -114,12 +114,12 @@ show_language_command (struct ui_file *file, int from_tty,
>    enum language flang;		/* The language of the frame.  */
>  
>    if (language_mode == language_mode_auto)
> -    fprintf_filtered (gdb_stdout,
> +    fprintf_filtered (file,
>  		      _("The current source language is "
>  			"\"auto; currently %s\".\n"),
>  		      current_language->name ());
>    else
> -    fprintf_filtered (gdb_stdout,
> +    fprintf_filtered (file,
>  		      _("The current source language is \"%s\".\n"),
>  		      current_language->name ());
>  
> @@ -132,7 +132,7 @@ show_language_command (struct ui_file *file, int from_tty,
>        if (flang != language_unknown
>  	  && language_mode == language_mode_manual
>  	  && current_language->la_language != flang)
> -	printf_filtered ("%s\n", _(lang_frame_mismatch_warn));
> +	fprintf_filtered (file, "%s\n", _(lang_frame_mismatch_warn));
>      }
>  }
>  
> @@ -220,12 +220,12 @@ show_range_command (struct ui_file *file, int from_tty,
>  			  "Unrecognized range check setting.");
>  	}
>  
> -      fprintf_filtered (gdb_stdout,
> +      fprintf_filtered (file,
>  			_("Range checking is \"auto; currently %s\".\n"),
>  			tmp);
>      }
>    else
> -    fprintf_filtered (gdb_stdout, _("Range checking is \"%s\".\n"),
> +    fprintf_filtered (file, _("Range checking is \"%s\".\n"),
>  		      value);
>  
>    if (range_check == range_check_warn
> @@ -296,13 +296,13 @@ show_case_command (struct ui_file *file, int from_tty,
>  			  "Unrecognized case-sensitive setting.");
>  	}
>  
> -      fprintf_filtered (gdb_stdout,
> +      fprintf_filtered (file,
>  			_("Case sensitivity in "
>  			  "name search is \"auto; currently %s\".\n"),
>  			tmp);
>      }
>    else
> -    fprintf_filtered (gdb_stdout,
> +    fprintf_filtered (file,
>  		      _("Case sensitivity in name search is \"%s\".\n"),
>  		      value);
>  
> diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
> index 603c5d2664b..d30487ae87b 100644
> --- a/gdb/mips-tdep.c
> +++ b/gdb/mips-tdep.c
> @@ -1190,14 +1190,15 @@ show_mask_address (struct ui_file *file, int from_tty,
>    switch (mask_address_var)
>      {
>      case AUTO_BOOLEAN_TRUE:
> -      printf_filtered ("The 32 bit mips address mask is enabled\n");
> +      fprintf_filtered (file, "The 32 bit mips address mask is enabled\n");
>        break;
>      case AUTO_BOOLEAN_FALSE:
> -      printf_filtered ("The 32 bit mips address mask is disabled\n");
> +      fprintf_filtered (file, "The 32 bit mips address mask is disabled\n");
>        break;
>      case AUTO_BOOLEAN_AUTO:
> -      printf_filtered
> -	("The 32 bit address mask is set automatically.  Currently %s\n",
> +      fprintf_filtered
> +	(file,
> +	 "The 32 bit address mask is set automatically.  Currently %s\n",
>  	 mips_mask_address_p (tdep) ? "enabled" : "disabled");
>        break;
>      default:
> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> index a6ce3db64e5..b2f07cb1870 100644
> --- a/gdb/record-btrace.c
> +++ b/gdb/record-btrace.c
> @@ -2956,7 +2956,7 @@ static void
>  cmd_show_replay_memory_access (struct ui_file *file, int from_tty,
>  			       struct cmd_list_element *c, const char *value)
>  {
> -  fprintf_filtered (gdb_stdout, _("Replay memory access is %s.\n"),
> +  fprintf_filtered (file, _("Replay memory access is %s.\n"),
>  		    replay_memory_access);
>  }
>  
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 46cd352ef90..78573d945d4 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -1065,8 +1065,6 @@ static int stub_unpack_int (const char *buff, int fieldlength);
>  
>  struct packet_config;
>  
> -static void show_packet_config_cmd (struct packet_config *config);
> -
>  static void show_remote_protocol_packet_cmd (struct ui_file *file,
>  					     int from_tty,
>  					     struct cmd_list_element *c,
> @@ -1913,7 +1911,7 @@ static enum packet_support packet_config_support (struct packet_config *config);
>  static enum packet_support packet_support (int packet);
>  
>  static void
> -show_packet_config_cmd (struct packet_config *config)
> +show_packet_config_cmd (ui_file *file, struct packet_config *config)
>  {
>    const char *support = "internal-error";
>  
> @@ -1932,14 +1930,16 @@ show_packet_config_cmd (struct packet_config *config)
>    switch (config->detect)
>      {
>      case AUTO_BOOLEAN_AUTO:
> -      printf_filtered (_("Support for the `%s' packet "
> -			 "is auto-detected, currently %s.\n"),
> -		       config->name, support);
> +      fprintf_filtered (file,
> +			_("Support for the `%s' packet "
> +			  "is auto-detected, currently %s.\n"),
> +			config->name, support);
>        break;
>      case AUTO_BOOLEAN_TRUE:
>      case AUTO_BOOLEAN_FALSE:
> -      printf_filtered (_("Support for the `%s' packet is currently %s.\n"),
> -		       config->name, support);
> +      fprintf_filtered (file,
> +			_("Support for the `%s' packet is currently %s.\n"),
> +			config->name, support);
>        break;
>      }
>  }
> @@ -2275,7 +2275,7 @@ show_remote_protocol_packet_cmd (struct ui_file *file, int from_tty,
>      {
>        if (c == packet->show_cmd)
>  	{
> -	  show_packet_config_cmd (packet);
> +	  show_packet_config_cmd (file, packet);
>  	  return;
>  	}
>      }
> @@ -2319,7 +2319,7 @@ show_remote_protocol_Z_packet_cmd (struct ui_file *file, int from_tty,
>  
>    for (i = 0; i < NR_Z_PACKET_TYPES; i++)
>      {
> -      show_packet_config_cmd (&remote_protocol_packets[PACKET_Z0 + i]);
> +      show_packet_config_cmd (file, &remote_protocol_packets[PACKET_Z0 + i]);
>      }
>  }
>  
> diff --git a/gdb/source.c b/gdb/source.c
> index 44e90bfab73..f2f116091b3 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -400,11 +400,11 @@ set_directories_command (const char *args,
>     function.  */
>  
>  static void
> -show_directories_1 (char *ignore, int from_tty)
> +show_directories_1 (ui_file *file, char *ignore, int from_tty)
>  {
> -  puts_filtered ("Source directories searched: ");
> -  puts_filtered (source_path.c_str ());
> -  puts_filtered ("\n");
> +  fputs_filtered ("Source directories searched: ", file);
> +  fputs_filtered (source_path.c_str (), file);
> +  fputs_filtered ("\n", file);
>  }
>  
>  /* Handler for "show directories" command.  */
> @@ -413,7 +413,7 @@ static void
>  show_directories_command (struct ui_file *file, int from_tty,
>  			  struct cmd_list_element *c, const char *value)
>  {
> -  show_directories_1 (NULL, from_tty);
> +  show_directories_1 (file, NULL, from_tty);
>  }
>  
>  /* See source.h.  */
> @@ -485,7 +485,7 @@ directory_command (const char *dirname, int from_tty)
>        gdb::observers::command_param_changed.notify ("directories",
>  						    source_path.c_str ());
>        if (from_tty)
> -	show_directories_1 ((char *) 0, from_tty);
> +	show_directories_1 (gdb_stdout, (char *) 0, from_tty);
>      }
>  }
>  
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index de1d68c0164..ffbfe355949 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -1303,11 +1303,13 @@ show_tdesc_filename_cmd (struct ui_file *file, int from_tty,
>    value = get_tdesc_info (current_inferior ())->filename.data ();
>  
>    if (value != NULL && *value != '\0')
> -    printf_filtered (_("The target description will be read from \"%s\".\n"),
> -		     value);
> +    fprintf_filtered (file,
> +		      _("The target description will be read from \"%s\".\n"),
> +		      value);
>    else
> -    printf_filtered (_("The target description will be "
> -		       "read from the target.\n"));
> +    fprintf_filtered (file,
> +		      _("The target description will be "
> +			"read from the target.\n"));
>  }
>  
>  static void
> diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
> index 8462d1eb1da..d92c5fcad18 100644
> --- a/gdb/tui/tui-win.c
> +++ b/gdb/tui/tui-win.c
> @@ -790,7 +790,7 @@ static void
>  tui_show_tab_width (struct ui_file *file, int from_tty,
>  		    struct cmd_list_element *c, const char *value)
>  {
> -  fprintf_filtered (gdb_stdout, _("TUI tab width is %s spaces.\n"), value);
> +  fprintf_filtered (file, _("TUI tab width is %s spaces.\n"), value);
>  
>  }
>  
> @@ -814,7 +814,7 @@ static void
>  tui_show_compact_source (struct ui_file *file, int from_tty,
>  			 struct cmd_list_element *c, const char *value)
>  {
> -  printf_filtered (_("TUI source window compactness is %s.\n"), value);
> +  fprintf_filtered (file, _("TUI source window compactness is %s.\n"), value);
>  }
>  
>  /* Set the tab width of the specified window.  */
> -- 
> 2.31.1
> 

-- 
Joel


More information about the Gdb-patches mailing list