This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA v2 22/24] Introduce gdb_argv, a class wrapper for buildargv
- From: Simon Marchi <simon dot marchi at polymtl dot ca>
- To: Tom Tromey <tom at tromey dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 31 Jul 2017 22:21:59 +0200
- Subject: Re: [RFA v2 22/24] Introduce gdb_argv, a class wrapper for buildargv
- Authentication-results: sourceware.org; auth=none
- References: <20170725172107.9799-1-tom@tromey.com> <20170725172107.9799-23-tom@tromey.com>
On 2017-07-25 19:21, Tom Tromey wrote:
@@ -254,11 +253,10 @@ gdbscm_string_to_argv (SCM string_scm)
return SCM_EOL;
}
- c_argv = gdb_buildargv (string);
+ gdb_argv c_argv (string);
for (i = 0; c_argv[i] != NULL; ++i)
Range-based for loop?
diff --git a/gdb/procfs.c b/gdb/procfs.c
index a0d2270..34e1996 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -5113,11 +5113,8 @@ procfs_info_proc (struct target_ops *ops, const
char *args,
}
old_chain = make_cleanup (null_cleanup, 0);
- if (args)
- {
- argv = gdb_buildargv (args);
- make_cleanup_freeargv (argv);
- }
+ gdb_argv built_argv (args);
+ argv = built_argv.get ();
while (argv != NULL && *argv != NULL)
An opportunity to use a range based for ?
@@ -890,7 +888,7 @@ pipe_windows_open (struct serial *scb, const char
*name)
const char *err_msg
= pex_run (ps->pex, PEX_SEARCH | PEX_BINARY_INPUT |
PEX_BINARY_OUTPUT
| PEX_STDERR_TO_PIPE,
- argv[0], argv, NULL, NULL,
+ argv[0], argv.get (), NULL, NULL,
&err);
if (err_msg)
@@ -920,6 +918,7 @@ pipe_windows_open (struct serial *scb, const char
*name)
scb->state = (void *) ps;
+ argv.release ();
This .release() seems to match previous behavior, but that previous
behavior looks fishy to me. Nothing seems to take ownership of that
argv. The only candidate would be pex_run, but as far as I can see it
doesn't.
diff --git a/gdb/stack.c b/gdb/stack.c
index 7f8a51c..9c6d87e 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1872,13 +1872,14 @@ backtrace_command (char *arg, int from_tty)
int fulltrace_arg = -1, arglen = 0, argc = 0, no_filters = -1;
int user_arg = 0;
+ gdb_argv built_argv;
Can this go in the "if" scope?
diff --git a/gdb/utils.c b/gdb/utils.c
index 06f4168..8a7a60f 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2863,11 +2863,25 @@ ldirname (const char *filename)
return dirname;
}
+/* See utils.h. */
+
+void
+gdb_argv::reset (const char *s)
+{
+ char **argv = buildargv (s);
+
+ if (s != NULL && argv == NULL)
+ malloc_failure (0);
+
+ freeargv (m_argv);
+ m_argv = argv;
+}
+
/* Call libiberty's buildargv, and return the result.
If buildargv fails due to out-of-memory, call nomem.
Therefore, the returned value is guaranteed to be non-NULL,
unless the parameter itself is NULL. */
-
+
Spurious space added here.
The patch LGTM (the .release() in the mingw code can be check later).
You can address the other comments at your discretion before pushing.
Thanks,
Simon