Re: [RFVv4 1/5] New cli-utils.h/.c function extract_info_print_args

[Pedro Alves <palves@redhat.com>]

Hi Philippe,

This is OK with the below fixed.

On 10/25/2018 09:11 PM, Philippe Waroquiers wrote:

> gdb/ChangeLog
> 2018-10-25  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* cli-utils.c (extract_arg_maybe_quoted): New function.
> 	(extract_info_print_args): New function.
> 	(info_print_args_help): New function.
>         (report_unrecognized_option_error): New function.
> 	* cli-utils.h (extract_arg_maybe_quoted): New function.
> 	(extract_info_print_args): New function.
> 	(info_print_args_help): New function.
>         (report_unrecognized_option_error): New function.

Noticed some leading whitespaces instead of tabs here.  Make
sure those are fixed when this copied to the ChangeLog file.

> +/* See documentation in cli-utils.h.  */
> +
> +const char *
> +info_print_args_help (const char *prefix,
> +		      const char *entity_kind)
> +{
> +  std::string help = string_printf (_("\
> +%sIf NAMEREGEXP is provided, only prints the %s whose name\n\
> +matches NAMEREGEXP.\n\
> +If -t TYPEREGEXP is provided, only prints the %s whose type\n\
> +matches TYPEREGEXP.  Note that the matching is done with the type\n\
> +printed by the 'whatis' command.\n\
> +By default, the command might produce headers and/or messages indicating\n\
> +why no %s can be printed.\n\
> +The flag -q disables the production of these headers and messages."),
> +				    prefix,
> +				    entity_kind, entity_kind, entity_kind);
> +
> +  /*  Note : this returns a string allocated with xstrdup, as this
> +      is typically used as argument to add_prefix_cmd, which needs a
> +      string that stays valid after destruction of the std::string.  */
> +  return xstrdup (help.c_str ());

Looks like you missed a previous comment here:

 "Keep the function's interface, but use xstrprintf instead
 of 'xstrdup + string_printf' then."

That meant you can skip the std::string and write this instead:

 const char *
 info_print_args_help (const char *prefix,
 		      const char *entity_kind)
 {
   return xstrprintf (_("\


> +/* Tell the user that ARGS starts with an option unrecognized by COMMAND,
> +   and throws an error.  */
> +

When I first read this comment I thought that the function printed to
gdb_stdout and then threw the error.  I'd suggest this tweak:

/* Throws an error telling the user that ARGS starts with an option
   unrecognized by COMMAND.  */

> +extern void report_unrecognized_option_error (const char *command,
> +					      const char *args);

Thanks,
Pedro Alves