Re: [PATCH 1/4] gdbsupport: Extend construct_inferior_arguments to allow handling all stringify_args cases

Christian Biesinger
2020-04-29T15:26:33Z

On Wed, Apr 29, 2020 at 6:17 AM Michael Weghorn via Gdb-patches
<gdb-patches@sourceware.org> wrote:
>
> Allow construct_inferior_arguments to handle zero args
> and have it return a std::string, similar to how
> stringify_argv in gdbsupport/common-utils does.
>
> Also, add a const qualifier for the second parameter,
> since it is only read, not written to.
>
> The intention is to replace some existing uses of
> stringify_argv by construct_inferior_arguments
> in a subsequent step, since construct_inferior_arguments
> properly handles special characters, while stringify_argv
> doesn't.

Can you clarify which revision your patch is against? My files look
nothing like the ones you are patching.

> 2020-04-29 Michael Weghorn <m.weghorn@posteo.de>
>
> * common-inferior.cc, common-inferior.h (construct_inferior_arguments):
> Adapt to handle zero args and return a std::string.
> Adapt call site.
> ---
> gdb/infcmd.c | 5 ++---
> gdbsupport/common-inferior.cc | 19 +++++++++++--------
> gdbsupport/common-inferior.h | 2 +-
> 3 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 8f7482347c..7ad931f9b4 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -151,12 +151,11 @@ get_inferior_args (void)
> {
> if (current_inferior ()->argc != 0)
> {
> - char *n;
> + std::string n;

While changing this, I would just move the declaration to the line
where it is first used.

>
> n = construct_inferior_arguments (current_inferior ()->argc,
> current_inferior ()->argv);
> - set_inferior_args (n);
> - xfree (n);
> + set_inferior_args (n.c_str());
> }
>
> if (current_inferior ()->args == NULL)
> diff --git a/gdbsupport/common-inferior.cc b/gdbsupport/common-inferior.cc
> index 71b9a11e02..3f117d5ef0 100644
> --- a/gdbsupport/common-inferior.cc
> +++ b/gdbsupport/common-inferior.cc
> @@ -28,15 +28,15 @@ bool startup_with_shell = true;
> /* Compute command-line string given argument vector. This does the
> same shell processing as fork_inferior. */
>
> -char *
> -construct_inferior_arguments (int argc, char **argv)
> +std::string
> +construct_inferior_arguments (int argc, char * const *argv)
> {
> - char *result;
> + gdb_assert (argc >= 0);
> + if (argc == 0 || argv[0] == NULL) {
> + return "";
> + }
>
> - /* ARGC should always be at least 1, but we double check this
> - here. This is also needed to silence -Werror-stringop
> - warnings. */
> - gdb_assert (argc > 0);
> + char *result;
>
> if (startup_with_shell)
> {
> @@ -145,5 +145,8 @@ construct_inferior_arguments (int argc, char **argv)
> }
> }
>
> - return result;
> + std::string str_result(result);
> + xfree (result);
> +
> + return str_result;
> }
> diff --git a/gdbsupport/common-inferior.h b/gdbsupport/common-inferior.h
> index eb60c8f13b..0b11e7d6a5 100644
> --- a/gdbsupport/common-inferior.h
> +++ b/gdbsupport/common-inferior.h
> @@ -58,6 +58,6 @@ extern void set_inferior_cwd (const char *cwd);
> the target is started up with a shell. */
> extern bool startup_with_shell;
>
> -extern char *construct_inferior_arguments (int, char **);
> +extern std::string construct_inferior_arguments (int, char * const *);
>
> #endif /* COMMON_COMMON_INFERIOR_H */
> --
> 2.26.2
>