* [RFA 02/23] Introduce and use gdb_file_up
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-02 17:08 ` Pedro Alves
2017-05-03 22:46 ` [RFA 09/23] Remove close cleanup Tom Tromey
` (22 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This introduces gdb_file_up, a unique pointer holding a FILE*, and
then changes some code in gdb to use it. In particular
gdb_fopen_cloexec now returns a gdb_file_up. This allow removing some
cleanups.
ChangeLog
2017-05-02 Tom Tromey <tom@tromey.com>
* xml-support.c (xml_fetch_content_from_file): Update.
* ui-file.c (stdio_file::open): Update.
* tracefile-tfile.c (tfile_start): Update.
* remote.c (remote_file_put, remote_file_get): Update.
* nat/linux-procfs.c (linux_proc_get_int)
(linux_proc_pid_get_state, linux_proc_tid_get_name): Update.
* nat/linux-osdata.c (linux_common_core_of_thread): Update.
(command_from_pid, commandline_from_pid, linux_xfer_osdata_cpus)
(print_sockets, linux_xfer_osdata_shm, linux_xfer_osdata_sem)
(linux_xfer_osdata_msg, linux_xfer_osdata_modules): Update.
* nat/linux-btrace.c (linux_determine_kernel_start): Update.
* linux-nat.c (linux_proc_pending_signals): Update.
* dwarf2read.c (write_psymtabs_to_index): Update.
* compile/compile.c (compile_to_object): Update.
* common/filestuff.h (struct gdb_file_deleter): New.
(gdb_file_up): New typedef.
(gdb_fopen_cloexec): Change return type.
* common/filestuff.c (gdb_fopen_cloexec): Return gdb_file_up.
* cli/cli-dump.c (fopen_with_cleanup): Remove.
(dump_binary_file, restore_binary_file): Update.
* auto-load.c (auto_load_objfile_script_1): Update.
---
gdb/ChangeLog | 24 ++++++++++++++++
gdb/auto-load.c | 7 ++---
gdb/cli/cli-dump.c | 28 +++++-------------
gdb/common/filestuff.c | 4 +--
gdb/common/filestuff.h | 15 +++++++++-
gdb/compile/compile.c | 17 +++++------
gdb/dwarf2read.c | 17 +++++------
gdb/linux-nat.c | 9 ++----
gdb/nat/linux-btrace.c | 9 ++----
gdb/nat/linux-osdata.c | 78 ++++++++++++++++----------------------------------
gdb/nat/linux-procfs.c | 18 ++++--------
gdb/remote.c | 18 +++++-------
gdb/tracefile-tfile.c | 2 +-
gdb/ui-file.c | 4 +--
gdb/xml-support.c | 13 ++++-----
15 files changed, 116 insertions(+), 147 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 843aef3..2bda6ff 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,29 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * xml-support.c (xml_fetch_content_from_file): Update.
+ * ui-file.c (stdio_file::open): Update.
+ * tracefile-tfile.c (tfile_start): Update.
+ * remote.c (remote_file_put, remote_file_get): Update.
+ * nat/linux-procfs.c (linux_proc_get_int)
+ (linux_proc_pid_get_state, linux_proc_tid_get_name): Update.
+ * nat/linux-osdata.c (linux_common_core_of_thread): Update.
+ (command_from_pid, commandline_from_pid, linux_xfer_osdata_cpus)
+ (print_sockets, linux_xfer_osdata_shm, linux_xfer_osdata_sem)
+ (linux_xfer_osdata_msg, linux_xfer_osdata_modules): Update.
+ * nat/linux-btrace.c (linux_determine_kernel_start): Update.
+ * linux-nat.c (linux_proc_pending_signals): Update.
+ * dwarf2read.c (write_psymtabs_to_index): Update.
+ * compile/compile.c (compile_to_object): Update.
+ * common/filestuff.h (struct gdb_file_deleter): New.
+ (gdb_file_up): New typedef.
+ (gdb_fopen_cloexec): Change return type.
+ * common/filestuff.c (gdb_fopen_cloexec): Return gdb_file_up.
+ * cli/cli-dump.c (fopen_with_cleanup): Remove.
+ (dump_binary_file, restore_binary_file): Update.
+ * auto-load.c (auto_load_objfile_script_1): Update.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* tracepoint.c (tvariables_info_1): Use ui_out_emit_table.
(info_static_tracepoint_markers_command): Likewise.
* solib.c (info_sharedlibrary_command): Use ui_out_emit_table.
diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 75ce828..292f2ae 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -786,7 +786,6 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,
{
char *filename, *debugfile;
int len, retval;
- FILE *input;
struct cleanup *cleanups;
const char *suffix = ext_lang_auto_load_suffix (language);
@@ -797,7 +796,7 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,
cleanups = make_cleanup (xfree, filename);
- input = gdb_fopen_cloexec (filename, "r");
+ gdb_file_up input = gdb_fopen_cloexec (filename, "r");
debugfile = filename;
if (debug_auto_load)
fprintf_unfiltered (gdb_stdlog, _("auto-load: Attempted file \"%s\" %s.\n"),
@@ -845,8 +844,6 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,
int is_safe;
struct auto_load_pspace_info *pspace_info;
- make_cleanup_fclose (input);
-
is_safe
= file_is_auto_load_safe (debugfile,
_("auto-load: Loading %s script \"%s\""
@@ -875,7 +872,7 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,
compiled in. And the extension language is required to implement
this function. */
gdb_assert (sourcer != NULL);
- sourcer (language, objfile, input, debugfile);
+ sourcer (language, objfile, input.get (), debugfile);
}
retval = 1;
diff --git a/gdb/cli/cli-dump.c b/gdb/cli/cli-dump.c
index 213622d..613ac1e 100644
--- a/gdb/cli/cli-dump.c
+++ b/gdb/cli/cli-dump.c
@@ -92,17 +92,6 @@ scan_filename_with_cleanup (const char **cmd, const char *defname)
return fullname;
}
-static FILE *
-fopen_with_cleanup (const char *filename, const char *mode)
-{
- FILE *file = gdb_fopen_cloexec (filename, mode);
-
- if (file == NULL)
- perror_with_name (filename);
- make_cleanup_fclose (file);
- return file;
-}
-
static gdb_bfd_ref_ptr
bfd_openr_or_error (const char *filename, const char *target)
{
@@ -168,11 +157,10 @@ static void
dump_binary_file (const char *filename, const char *mode,
const bfd_byte *buf, ULONGEST len)
{
- FILE *file;
int status;
- file = fopen_with_cleanup (filename, mode);
- status = fwrite (buf, len, 1, file);
+ gdb_file_up file = gdb_fopen_cloexec (filename, mode);
+ status = fwrite (buf, len, 1, file.get ());
if (status != 1)
perror_with_name (filename);
}
@@ -509,14 +497,13 @@ restore_section_callback (bfd *ibfd, asection *isec, void *args)
static void
restore_binary_file (const char *filename, struct callback_data *data)
{
- struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
- FILE *file = fopen_with_cleanup (filename, FOPEN_RB);
+ gdb_file_up file = gdb_fopen_cloexec (filename, FOPEN_RB);
long len;
/* Get the file size for reading. */
- if (fseek (file, 0, SEEK_END) == 0)
+ if (fseek (file.get (), 0, SEEK_END) == 0)
{
- len = ftell (file);
+ len = ftell (file.get ());
if (len < 0)
perror_with_name (filename);
}
@@ -541,12 +528,12 @@ restore_binary_file (const char *filename, struct callback_data *data)
(unsigned long) (data->load_start + data->load_offset + len));
/* Now set the file pos to the requested load start pos. */
- if (fseek (file, data->load_start, SEEK_SET) != 0)
+ if (fseek (file.get (), data->load_start, SEEK_SET) != 0)
perror_with_name (filename);
/* Now allocate a buffer and read the file contents. */
std::unique_ptr<gdb_byte[]> buf (new gdb_byte[len]);
- if (fread (buf.get (), 1, len, file) != len)
+ if (fread (buf.get (), 1, len, file.get ()) != len)
perror_with_name (filename);
/* Now write the buffer into target memory. */
@@ -554,7 +541,6 @@ restore_binary_file (const char *filename, struct callback_data *data)
buf.get (), len);
if (len != 0)
warning (_("restore: memory write failed (%s)."), safe_strerror (len));
- do_cleanups (cleanup);
}
static void
diff --git a/gdb/common/filestuff.c b/gdb/common/filestuff.c
index 725fb92..4b05884 100644
--- a/gdb/common/filestuff.c
+++ b/gdb/common/filestuff.c
@@ -300,7 +300,7 @@ gdb_open_cloexec (const char *filename, int flags, unsigned long mode)
/* See filestuff.h. */
-FILE *
+gdb_file_up
gdb_fopen_cloexec (const char *filename, const char *opentype)
{
FILE *result;
@@ -336,7 +336,7 @@ gdb_fopen_cloexec (const char *filename, const char *opentype)
if (result != NULL)
maybe_mark_cloexec (fileno (result));
- return result;
+ return gdb_file_up (result);
}
#ifdef HAVE_SOCKETS
diff --git a/gdb/common/filestuff.h b/gdb/common/filestuff.h
index b0a811b..3cf2df6 100644
--- a/gdb/common/filestuff.h
+++ b/gdb/common/filestuff.h
@@ -46,10 +46,23 @@ extern void close_most_fds (void);
extern int gdb_open_cloexec (const char *filename, int flags,
/* mode_t */ unsigned long mode);
+struct gdb_file_deleter
+{
+ void operator() (FILE *file) const
+ {
+ fclose (file);
+ }
+};
+
+/* A unique pointer to a FILE. */
+
+typedef std::unique_ptr<FILE, gdb_file_deleter> gdb_file_up;
+
/* Like 'fopen', but ensures that the returned file descriptor has the
close-on-exec flag set. */
-extern FILE *gdb_fopen_cloexec (const char *filename, const char *opentype);
+extern gdb_file_up gdb_fopen_cloexec (const char *filename,
+ const char *opentype);
/* Like 'socketpair', but ensures that the returned file descriptors
have the close-on-exec flag set. */
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 1771692..d8c505f 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -547,14 +547,15 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
compile_file_names fnames = get_new_file_names ();
- src = gdb_fopen_cloexec (fnames.source_file (), "w");
- if (src == NULL)
- perror_with_name (_("Could not open source file for writing"));
- inner_cleanup = make_cleanup (cleanup_unlink_file,
- (void *) fnames.source_file ());
- if (fputs (code.c_str (), src) == EOF)
- perror_with_name (_("Could not write to source file"));
- fclose (src);
+ {
+ gdb_file_up src = gdb_fopen_cloexec (fnames.source_file (), "w");
+ if (src == NULL)
+ perror_with_name (_("Could not open source file for writing"));
+ inner_cleanup = make_cleanup (cleanup_unlink_file,
+ (void *) fnames.source_file ());
+ if (fputs (code.c_str (), src.get ()) == EOF)
+ perror_with_name (_("Could not write to source file"));
+ }
if (compile_debug)
fprintf_unfiltered (gdb_stdlog, "source file produced: %s\n\n",
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index b58d0fc..321743d 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -23873,7 +23873,6 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir)
struct obstack contents, addr_obstack, constant_pool, symtab_obstack;
struct obstack cu_list, types_cu_list;
int i;
- FILE *out_file;
struct mapped_symtab *symtab;
offset_type val, size_of_contents, total_len;
struct stat st;
@@ -23895,7 +23894,7 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir)
INDEX_SUFFIX, (char *) NULL);
cleanup = make_cleanup (xfree, filename);
- out_file = gdb_fopen_cloexec (filename, "wb");
+ gdb_file_up out_file = gdb_fopen_cloexec (filename, "wb");
if (!out_file)
error (_("Can't open `%s' for writing"), filename);
@@ -24029,14 +24028,12 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir)
gdb_assert (obstack_object_size (&contents) == size_of_contents);
- write_obstack (out_file, &contents);
- write_obstack (out_file, &cu_list);
- write_obstack (out_file, &types_cu_list);
- write_obstack (out_file, &addr_obstack);
- write_obstack (out_file, &symtab_obstack);
- write_obstack (out_file, &constant_pool);
-
- fclose (out_file);
+ write_obstack (out_file.get (), &contents);
+ write_obstack (out_file.get (), &cu_list);
+ write_obstack (out_file.get (), &types_cu_list);
+ write_obstack (out_file.get (), &addr_obstack);
+ write_obstack (out_file.get (), &symtab_obstack);
+ write_obstack (out_file.get (), &constant_pool);
/* We want to keep the file. */
unlink_file.keep ();
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index b008df0..fd567bf 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4187,20 +4187,17 @@ void
linux_proc_pending_signals (int pid, sigset_t *pending,
sigset_t *blocked, sigset_t *ignored)
{
- FILE *procfile;
char buffer[PATH_MAX], fname[PATH_MAX];
- struct cleanup *cleanup;
sigemptyset (pending);
sigemptyset (blocked);
sigemptyset (ignored);
xsnprintf (fname, sizeof fname, "/proc/%d/status", pid);
- procfile = gdb_fopen_cloexec (fname, "r");
+ gdb_file_up procfile = gdb_fopen_cloexec (fname, "r");
if (procfile == NULL)
error (_("Could not open %s"), fname);
- cleanup = make_cleanup_fclose (procfile);
- while (fgets (buffer, PATH_MAX, procfile) != NULL)
+ while (fgets (buffer, PATH_MAX, procfile.get ()) != NULL)
{
/* Normal queued signals are on the SigPnd line in the status
file. However, 2.6 kernels also have a "shared" pending
@@ -4219,8 +4216,6 @@ linux_proc_pending_signals (int pid, sigset_t *pending,
else if (startswith (buffer, "SigIgn:\t"))
add_line_to_sigset (buffer + 8, ignored);
}
-
- do_cleanups (cleanup);
}
static enum target_xfer_status
diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c
index b2c84c1..1a383b7 100644
--- a/gdb/nat/linux-btrace.c
+++ b/gdb/nat/linux-btrace.c
@@ -204,24 +204,23 @@ linux_determine_kernel_start (void)
{
static uint64_t kernel_start;
static int cached;
- FILE *file;
if (cached != 0)
return kernel_start;
cached = 1;
- file = gdb_fopen_cloexec ("/proc/kallsyms", "r");
+ gdb_file_up file = gdb_fopen_cloexec ("/proc/kallsyms", "r");
if (file == NULL)
return kernel_start;
- while (!feof (file))
+ while (!feof (file.get ()))
{
char buffer[1024], symbol[8], *line;
uint64_t addr;
int match;
- line = fgets (buffer, sizeof (buffer), file);
+ line = fgets (buffer, sizeof (buffer), file.get ());
if (line == NULL)
break;
@@ -236,8 +235,6 @@ linux_determine_kernel_start (void)
}
}
- fclose (file);
-
return kernel_start;
}
diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
index ba2e5a5..4b40a4d 100644
--- a/gdb/nat/linux-osdata.c
+++ b/gdb/nat/linux-osdata.c
@@ -61,7 +61,6 @@ int
linux_common_core_of_thread (ptid_t ptid)
{
char filename[sizeof ("/proc//task//stat") + 2 * MAX_PID_T_STRLEN];
- FILE *f;
char *content = NULL;
char *p;
char *ts = 0;
@@ -71,7 +70,7 @@ linux_common_core_of_thread (ptid_t ptid)
sprintf (filename, "/proc/%lld/task/%lld/stat",
(PID_T) ptid_get_pid (ptid), (PID_T) ptid_get_lwp (ptid));
- f = gdb_fopen_cloexec (filename, "r");
+ gdb_file_up f = gdb_fopen_cloexec (filename, "r");
if (!f)
return -1;
@@ -79,7 +78,7 @@ linux_common_core_of_thread (ptid_t ptid)
{
int n;
content = (char *) xrealloc (content, content_read + 1024);
- n = fread (content + content_read, 1, 1024, f);
+ n = fread (content + content_read, 1, 1024, f.get ());
content_read += n;
if (n < 1024)
{
@@ -104,7 +103,6 @@ linux_common_core_of_thread (ptid_t ptid)
core = -1;
xfree (content);
- fclose (f);
return core;
}
@@ -117,7 +115,7 @@ static void
command_from_pid (char *command, int maxlen, PID_T pid)
{
char *stat_path = xstrprintf ("/proc/%lld/stat", pid);
- FILE *fp = gdb_fopen_cloexec (stat_path, "r");
+ gdb_file_up fp = gdb_fopen_cloexec (stat_path, "r");
command[0] = '\0';
@@ -128,15 +126,13 @@ command_from_pid (char *command, int maxlen, PID_T pid)
(for the brackets). */
char cmd[18];
PID_T stat_pid;
- int items_read = fscanf (fp, "%lld %17s", &stat_pid, cmd);
+ int items_read = fscanf (fp.get (), "%lld %17s", &stat_pid, cmd);
if (items_read == 2 && pid == stat_pid)
{
cmd[strlen (cmd) - 1] = '\0'; /* Remove trailing parenthesis. */
strncpy (command, cmd + 1, maxlen); /* Ignore leading parenthesis. */
}
-
- fclose (fp);
}
else
{
@@ -157,16 +153,16 @@ commandline_from_pid (PID_T pid)
{
char *pathname = xstrprintf ("/proc/%lld/cmdline", pid);
char *commandline = NULL;
- FILE *f = gdb_fopen_cloexec (pathname, "r");
+ gdb_file_up f = gdb_fopen_cloexec (pathname, "r");
if (f)
{
size_t len = 0;
- while (!feof (f))
+ while (!feof (f.get ()))
{
char buf[1024];
- size_t read_bytes = fread (buf, 1, sizeof (buf), f);
+ size_t read_bytes = fread (buf, 1, sizeof (buf), f.get ());
if (read_bytes)
{
@@ -176,8 +172,6 @@ commandline_from_pid (PID_T pid)
}
}
- fclose (f);
-
if (commandline)
{
size_t i;
@@ -675,7 +669,6 @@ linux_xfer_osdata_cpus (gdb_byte *readbuf,
if (offset == 0)
{
- FILE *fp;
int first_item = 1;
if (len_avail != -1 && len_avail != 0)
@@ -685,14 +678,14 @@ linux_xfer_osdata_cpus (gdb_byte *readbuf,
buffer_init (&buffer);
buffer_grow_str (&buffer, "<osdata type=\"cpus\">\n");
- fp = gdb_fopen_cloexec ("/proc/cpuinfo", "r");
+ gdb_file_up fp = gdb_fopen_cloexec ("/proc/cpuinfo", "r");
if (fp != NULL)
{
char buf[8192];
do
{
- if (fgets (buf, sizeof (buf), fp))
+ if (fgets (buf, sizeof (buf), fp.get ()))
{
char *key, *value;
int i = 0;
@@ -732,12 +725,10 @@ linux_xfer_osdata_cpus (gdb_byte *readbuf,
value);
}
}
- while (!feof (fp));
+ while (!feof (fp.get ()));
if (first_item == 0)
buffer_grow_str (&buffer, "</item>");
-
- fclose (fp);
}
buffer_grow_str0 (&buffer, "</osdata>\n");
@@ -942,7 +933,6 @@ static void
print_sockets (unsigned short family, int tcp, struct buffer *buffer)
{
const char *proc_file;
- FILE *fp;
if (family == AF_INET)
proc_file = tcp ? "/proc/net/tcp" : "/proc/net/udp";
@@ -951,14 +941,14 @@ print_sockets (unsigned short family, int tcp, struct buffer *buffer)
else
return;
- fp = gdb_fopen_cloexec (proc_file, "r");
+ gdb_file_up fp = gdb_fopen_cloexec (proc_file, "r");
if (fp)
{
char buf[8192];
do
{
- if (fgets (buf, sizeof (buf), fp))
+ if (fgets (buf, sizeof (buf), fp.get ()))
{
uid_t uid;
unsigned int local_port, remote_port, state;
@@ -1064,9 +1054,7 @@ print_sockets (unsigned short family, int tcp, struct buffer *buffer)
}
}
}
- while (!feof (fp));
-
- fclose (fp);
+ while (!feof (fp.get ()));
}
}
@@ -1163,8 +1151,6 @@ linux_xfer_osdata_shm (gdb_byte *readbuf,
if (offset == 0)
{
- FILE *fp;
-
if (len_avail != -1 && len_avail != 0)
buffer_free (&buffer);
len_avail = 0;
@@ -1172,14 +1158,14 @@ linux_xfer_osdata_shm (gdb_byte *readbuf,
buffer_init (&buffer);
buffer_grow_str (&buffer, "<osdata type=\"shared memory\">\n");
- fp = gdb_fopen_cloexec ("/proc/sysvipc/shm", "r");
+ gdb_file_up fp = gdb_fopen_cloexec ("/proc/sysvipc/shm", "r");
if (fp)
{
char buf[8192];
do
{
- if (fgets (buf, sizeof (buf), fp))
+ if (fgets (buf, sizeof (buf), fp.get ()))
{
key_t key;
uid_t uid, cuid;
@@ -1252,9 +1238,7 @@ linux_xfer_osdata_shm (gdb_byte *readbuf,
}
}
}
- while (!feof (fp));
-
- fclose (fp);
+ while (!feof (fp.get ()));
}
buffer_grow_str0 (&buffer, "</osdata>\n");
@@ -1291,8 +1275,6 @@ linux_xfer_osdata_sem (gdb_byte *readbuf,
if (offset == 0)
{
- FILE *fp;
-
if (len_avail != -1 && len_avail != 0)
buffer_free (&buffer);
len_avail = 0;
@@ -1300,14 +1282,14 @@ linux_xfer_osdata_sem (gdb_byte *readbuf,
buffer_init (&buffer);
buffer_grow_str (&buffer, "<osdata type=\"semaphores\">\n");
- fp = gdb_fopen_cloexec ("/proc/sysvipc/sem", "r");
+ gdb_file_up fp = gdb_fopen_cloexec ("/proc/sysvipc/sem", "r");
if (fp)
{
char buf[8192];
do
{
- if (fgets (buf, sizeof (buf), fp))
+ if (fgets (buf, sizeof (buf), fp.get ()))
{
key_t key;
uid_t uid, cuid;
@@ -1364,9 +1346,7 @@ linux_xfer_osdata_sem (gdb_byte *readbuf,
}
}
}
- while (!feof (fp));
-
- fclose (fp);
+ while (!feof (fp.get ()));
}
buffer_grow_str0 (&buffer, "</osdata>\n");
@@ -1403,8 +1383,6 @@ linux_xfer_osdata_msg (gdb_byte *readbuf,
if (offset == 0)
{
- FILE *fp;
-
if (len_avail != -1 && len_avail != 0)
buffer_free (&buffer);
len_avail = 0;
@@ -1412,14 +1390,14 @@ linux_xfer_osdata_msg (gdb_byte *readbuf,
buffer_init (&buffer);
buffer_grow_str (&buffer, "<osdata type=\"message queues\">\n");
- fp = gdb_fopen_cloexec ("/proc/sysvipc/msg", "r");
+ gdb_file_up fp = gdb_fopen_cloexec ("/proc/sysvipc/msg", "r");
if (fp)
{
char buf[8192];
do
{
- if (fgets (buf, sizeof (buf), fp))
+ if (fgets (buf, sizeof (buf), fp.get ()))
{
key_t key;
PID_T lspid, lrpid;
@@ -1490,9 +1468,7 @@ linux_xfer_osdata_msg (gdb_byte *readbuf,
}
}
}
- while (!feof (fp));
-
- fclose (fp);
+ while (!feof (fp.get ()));
}
buffer_grow_str0 (&buffer, "</osdata>\n");
@@ -1529,8 +1505,6 @@ linux_xfer_osdata_modules (gdb_byte *readbuf,
if (offset == 0)
{
- FILE *fp;
-
if (len_avail != -1 && len_avail != 0)
buffer_free (&buffer);
len_avail = 0;
@@ -1538,14 +1512,14 @@ linux_xfer_osdata_modules (gdb_byte *readbuf,
buffer_init (&buffer);
buffer_grow_str (&buffer, "<osdata type=\"modules\">\n");
- fp = gdb_fopen_cloexec ("/proc/modules", "r");
+ gdb_file_up fp = gdb_fopen_cloexec ("/proc/modules", "r");
if (fp)
{
char buf[8192];
do
{
- if (fgets (buf, sizeof (buf), fp))
+ if (fgets (buf, sizeof (buf), fp.get ()))
{
char *name, *dependencies, *status, *tmp;
unsigned int size;
@@ -1600,9 +1574,7 @@ linux_xfer_osdata_modules (gdb_byte *readbuf,
address);
}
}
- while (!feof (fp));
-
- fclose (fp);
+ while (!feof (fp.get ()));
}
buffer_grow_str0 (&buffer, "</osdata>\n");
diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
index 5290045..a12f622 100644
--- a/gdb/nat/linux-procfs.c
+++ b/gdb/nat/linux-procfs.c
@@ -29,12 +29,11 @@ static int
linux_proc_get_int (pid_t lwpid, const char *field, int warn)
{
size_t field_len = strlen (field);
- FILE *status_file;
char buf[100];
int retval = -1;
snprintf (buf, sizeof (buf), "/proc/%d/status", (int) lwpid);
- status_file = gdb_fopen_cloexec (buf, "r");
+ gdb_file_up status_file = gdb_fopen_cloexec (buf, "r");
if (status_file == NULL)
{
if (warn)
@@ -42,14 +41,13 @@ linux_proc_get_int (pid_t lwpid, const char *field, int warn)
return -1;
}
- while (fgets (buf, sizeof (buf), status_file))
+ while (fgets (buf, sizeof (buf), status_file.get ()))
if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':')
{
retval = strtol (&buf[field_len + 1], NULL, 10);
break;
}
- fclose (status_file);
return retval;
}
@@ -128,12 +126,11 @@ parse_proc_status_state (const char *state)
static int
linux_proc_pid_get_state (pid_t pid, int warn, enum proc_state *state)
{
- FILE *procfile;
int have_state;
char buffer[100];
xsnprintf (buffer, sizeof (buffer), "/proc/%d/status", (int) pid);
- procfile = gdb_fopen_cloexec (buffer, "r");
+ gdb_file_up procfile = gdb_fopen_cloexec (buffer, "r");
if (procfile == NULL)
{
if (warn)
@@ -142,14 +139,13 @@ linux_proc_pid_get_state (pid_t pid, int warn, enum proc_state *state)
}
have_state = 0;
- while (fgets (buffer, sizeof (buffer), procfile) != NULL)
+ while (fgets (buffer, sizeof (buffer), procfile.get ()) != NULL)
if (startswith (buffer, "State:"))
{
have_state = 1;
*state = parse_proc_status_state (buffer + sizeof ("State:") - 1);
break;
}
- fclose (procfile);
return have_state;
}
@@ -242,7 +238,6 @@ linux_proc_tid_get_name (ptid_t ptid)
static char comm_buf[TASK_COMM_LEN];
char comm_path[100];
- FILE *comm_file;
const char *comm_val;
pid_t pid = ptid_get_pid (ptid);
pid_t tid = ptid_lwp_p (ptid) ? ptid_get_lwp (ptid) : ptid_get_pid (ptid);
@@ -250,12 +245,11 @@ linux_proc_tid_get_name (ptid_t ptid)
xsnprintf (comm_path, sizeof (comm_path),
"/proc/%ld/task/%ld/comm", (long) pid, (long) tid);
- comm_file = gdb_fopen_cloexec (comm_path, "r");
+ gdb_file_up comm_file = gdb_fopen_cloexec (comm_path, "r");
if (comm_file == NULL)
return NULL;
- comm_val = fgets (comm_buf, sizeof (comm_buf), comm_file);
- fclose (comm_file);
+ comm_val = fgets (comm_buf, sizeof (comm_buf), comm_file.get ());
if (comm_val != NULL)
{
diff --git a/gdb/remote.c b/gdb/remote.c
index 2cd9850..1db2e57 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11893,7 +11893,6 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty)
{
struct cleanup *back_to, *close_cleanup;
int retcode, fd, remote_errno, bytes, io_size;
- FILE *file;
gdb_byte *buffer;
int bytes_in_buffer;
int saw_eof;
@@ -11903,10 +11902,9 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty)
if (!rs->remote_desc)
error (_("command can only be used with remote target"));
- file = gdb_fopen_cloexec (local_file, "rb");
+ gdb_file_up file = gdb_fopen_cloexec (local_file, "rb");
if (file == NULL)
perror_with_name (local_file);
- back_to = make_cleanup_fclose (file);
fd = remote_hostio_open (find_target_at (process_stratum), NULL,
remote_file, (FILEIO_O_WRONLY | FILEIO_O_CREAT
@@ -11919,7 +11917,7 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty)
remote packet limit, so we'll transfer slightly fewer. */
io_size = get_remote_packet_size ();
buffer = (gdb_byte *) xmalloc (io_size);
- make_cleanup (xfree, buffer);
+ back_to = make_cleanup (xfree, buffer);
close_cleanup = make_cleanup (remote_hostio_close_cleanup, &fd);
@@ -11932,10 +11930,10 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty)
{
bytes = fread (buffer + bytes_in_buffer, 1,
io_size - bytes_in_buffer,
- file);
+ file.get ());
if (bytes == 0)
{
- if (ferror (file))
+ if (ferror (file.get ()))
error (_("Error reading %s."), local_file);
else
{
@@ -11986,7 +11984,6 @@ remote_file_get (const char *remote_file, const char *local_file, int from_tty)
{
struct cleanup *back_to, *close_cleanup;
int fd, remote_errno, bytes, io_size;
- FILE *file;
gdb_byte *buffer;
ULONGEST offset;
struct remote_state *rs = get_remote_state ();
@@ -12000,16 +11997,15 @@ remote_file_get (const char *remote_file, const char *local_file, int from_tty)
if (fd == -1)
remote_hostio_error (remote_errno);
- file = gdb_fopen_cloexec (local_file, "wb");
+ gdb_file_up file = gdb_fopen_cloexec (local_file, "wb");
if (file == NULL)
perror_with_name (local_file);
- back_to = make_cleanup_fclose (file);
/* Send up to this many bytes at once. They won't all fit in the
remote packet limit, so we'll transfer slightly fewer. */
io_size = get_remote_packet_size ();
buffer = (gdb_byte *) xmalloc (io_size);
- make_cleanup (xfree, buffer);
+ back_to = make_cleanup (xfree, buffer);
close_cleanup = make_cleanup (remote_hostio_close_cleanup, &fd);
@@ -12026,7 +12022,7 @@ remote_file_get (const char *remote_file, const char *local_file, int from_tty)
offset += bytes;
- bytes = fwrite (buffer, 1, bytes, file);
+ bytes = fwrite (buffer, 1, bytes, file.get ());
if (bytes == 0)
perror_with_name (local_file);
}
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 255bbc9..c58ac46 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -89,7 +89,7 @@ tfile_start (struct trace_file_writer *self, const char *filename)
= (struct tfile_trace_file_writer *) self;
writer->pathname = tilde_expand (filename);
- writer->fp = gdb_fopen_cloexec (writer->pathname, "wb");
+ writer->fp = gdb_fopen_cloexec (writer->pathname, "wb").release ();
if (writer->fp == NULL)
error (_("Unable to open file '%s' for saving trace data (%s)"),
writer->pathname, safe_strerror (errno));
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index 60e3274..046e14f 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -177,12 +177,12 @@ stdio_file::open (const char *name, const char *mode)
m_close_p = false;
}
- FILE *f = gdb_fopen_cloexec (name, mode);
+ gdb_file_up f = gdb_fopen_cloexec (name, mode);
if (f == NULL)
return false;
- set_stream (f);
+ set_stream (f.release ());
m_close_p = true;
return true;
diff --git a/gdb/xml-support.c b/gdb/xml-support.c
index fff3997..65638b9 100644
--- a/gdb/xml-support.c
+++ b/gdb/xml-support.c
@@ -1001,7 +1001,7 @@ char *
xml_fetch_content_from_file (const char *filename, void *baton)
{
const char *dirname = (const char *) baton;
- FILE *file;
+ gdb_file_up file;
struct cleanup *back_to;
char *text;
size_t len, offset;
@@ -1021,21 +1021,19 @@ xml_fetch_content_from_file (const char *filename, void *baton)
if (file == NULL)
return NULL;
- back_to = make_cleanup_fclose (file);
-
/* Read in the whole file, one chunk at a time. */
len = 4096;
offset = 0;
text = (char *) xmalloc (len);
- make_cleanup (free_current_contents, &text);
+ back_to = make_cleanup (free_current_contents, &text);
while (1)
{
size_t bytes_read;
/* Continue reading where the last read left off. Leave at least
one byte so that we can NUL-terminate the result. */
- bytes_read = fread (text + offset, 1, len - offset - 1, file);
- if (ferror (file))
+ bytes_read = fread (text + offset, 1, len - offset - 1, file.get ());
+ if (ferror (file.get ()))
{
warning (_("Read error from \"%s\""), filename);
do_cleanups (back_to);
@@ -1044,14 +1042,13 @@ xml_fetch_content_from_file (const char *filename, void *baton)
offset += bytes_read;
- if (feof (file))
+ if (feof (file.get ()))
break;
len = len * 2;
text = (char *) xrealloc (text, len);
}
- fclose (file);
discard_cleanups (back_to);
text[offset] = '\0';
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* [RFA 09/23] Remove close cleanup
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
2017-05-03 22:46 ` [RFA 02/23] Introduce and use gdb_file_up Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-02 18:08 ` Pedro Alves
2017-05-03 22:46 ` [RFA 23/23] Use gdb_argv_up in Python Tom Tromey
` (21 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
make_cleanup_close calls close on a file descriptor. This patch
replaces it with a new RAII class, gdb::fd_closer.
2017-05-02 Tom Tromey <tom@tromey.com>
* source.c (get_filename_and_charpos, forward_search_command)
(reverse_search_command): Use fd_closer.
* procfs.c (load_syscalls, proc_get_LDT_entry)
(iterate_over_mappings): Use fd_closer.
* nto-procfs.c (procfs_open_1, procfs_pidlist): Use fd_closer.
* nat/linux-namespaces.c (linux_mntns_access_fs): Use fd_closer.
* common/filestuff.h (gdb::fd_closer): New class.
* common/filestuff.c (do_close_cleanup, make_cleanup_close):
Remove.
---
gdb/ChangeLog | 12 +++++++
gdb/common/filestuff.c | 21 ------------
gdb/common/filestuff.h | 39 ++++++++++++++++++++--
gdb/nat/linux-namespaces.c | 80 +++++++++++++++++++++-------------------------
gdb/nto-procfs.c | 9 ++----
gdb/procfs.c | 26 ++++++---------
gdb/source.c | 13 ++++----
7 files changed, 103 insertions(+), 97 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2d0d71d..50d8794 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,17 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * source.c (get_filename_and_charpos, forward_search_command)
+ (reverse_search_command): Use fd_closer.
+ * procfs.c (load_syscalls, proc_get_LDT_entry)
+ (iterate_over_mappings): Use fd_closer.
+ * nto-procfs.c (procfs_open_1, procfs_pidlist): Use fd_closer.
+ * nat/linux-namespaces.c (linux_mntns_access_fs): Use fd_closer.
+ * common/filestuff.h (gdb::fd_closer): New class.
+ * common/filestuff.c (do_close_cleanup, make_cleanup_close):
+ Remove.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* compile/compile.c (cleanup_unlink_file): Remove.
(compile_to_object): Use gdb::unlinker.
(eval_compile_command): Likewise.
diff --git a/gdb/common/filestuff.c b/gdb/common/filestuff.c
index 4b05884..5a0dd65 100644
--- a/gdb/common/filestuff.c
+++ b/gdb/common/filestuff.c
@@ -404,24 +404,3 @@ gdb_pipe_cloexec (int filedes[2])
return result;
}
-
-/* Helper function which does the work for make_cleanup_close. */
-
-static void
-do_close_cleanup (void *arg)
-{
- int *fd = (int *) arg;
-
- close (*fd);
-}
-
-/* See filestuff.h. */
-
-struct cleanup *
-make_cleanup_close (int fd)
-{
- int *saved_fd = XNEW (int);
-
- *saved_fd = fd;
- return make_cleanup_dtor (do_close_cleanup, saved_fd, xfree);
-}
diff --git a/gdb/common/filestuff.h b/gdb/common/filestuff.h
index 3cf2df6..82eb5b3 100644
--- a/gdb/common/filestuff.h
+++ b/gdb/common/filestuff.h
@@ -80,8 +80,43 @@ extern int gdb_socket_cloexec (int domain, int style, int protocol);
extern int gdb_pipe_cloexec (int filedes[2]);
-/* Return a new cleanup that closes FD. */
+namespace gdb
+{
+
+/* A class that manages a file descriptor and closes it on
+ destruction. */
+
+class fd_closer
+{
+public:
+
+ explicit fd_closer (int fd)
+ : m_fd (fd)
+ {
+ }
+
+ ~fd_closer ()
+ {
+ if (m_fd != -1)
+ close (m_fd);
+ }
+
+ fd_closer (const fd_closer &) = delete;
+ fd_closer &operator= (const fd_closer &) = delete;
+
+ /* Release the file descriptor to the caller. */
+ int release ()
+ {
+ int result = m_fd;
+ m_fd = -1;
+ return result;
+ }
+
+private:
+
+ int m_fd;
+};
-extern struct cleanup *make_cleanup_close (int fd);
+}
#endif /* FILESTUFF_H */
diff --git a/gdb/nat/linux-namespaces.c b/gdb/nat/linux-namespaces.c
index 1df8f1b..fcc7ba3 100644
--- a/gdb/nat/linux-namespaces.c
+++ b/gdb/nat/linux-namespaces.c
@@ -887,7 +887,6 @@ enum mnsh_fs_code
static enum mnsh_fs_code
linux_mntns_access_fs (pid_t pid)
{
- struct cleanup *old_chain;
struct linux_ns *ns;
struct stat sb;
struct linux_mnsh *helper;
@@ -901,64 +900,59 @@ linux_mntns_access_fs (pid_t pid)
if (ns == NULL)
return MNSH_FS_DIRECT;
- old_chain = make_cleanup (null_cleanup, NULL);
-
fd = gdb_open_cloexec (linux_ns_filename (ns, pid), O_RDONLY, 0);
if (fd < 0)
- goto error;
-
- make_cleanup_close (fd);
+ return MNSH_FS_ERROR;
- if (fstat (fd, &sb) != 0)
- goto error;
+ /* Introduce a new scope here so we can reset errno after
+ closing. */
+ {
+ gdb::fd_closer close_fd (fd);
- if (sb.st_ino == ns->id)
- {
- do_cleanups (old_chain);
+ if (fstat (fd, &sb) != 0)
+ goto error;
+ if (sb.st_ino == ns->id)
return MNSH_FS_DIRECT;
- }
-
- helper = linux_mntns_get_helper ();
- if (helper == NULL)
- goto error;
- if (sb.st_ino != helper->nsid)
- {
- int result, error;
+ helper = linux_mntns_get_helper ();
+ if (helper == NULL)
+ goto error;
- size = mnsh_send_setns (helper, fd, 0);
- if (size < 0)
- goto error;
+ if (sb.st_ino != helper->nsid)
+ {
+ int result, error;
- if (mnsh_recv_int (helper, &result, &error) != 0)
- goto error;
+ size = mnsh_send_setns (helper, fd, 0);
+ if (size < 0)
+ goto error;
- if (result != 0)
- {
- /* ENOSYS indicates that an entire function is unsupported
- (it's not appropriate for our versions of open/unlink/
- readlink to sometimes return with ENOSYS depending on how
- they're called) so we convert ENOSYS to ENOTSUP if setns
- fails. */
- if (error == ENOSYS)
- error = ENOTSUP;
-
- errno = error;
+ if (mnsh_recv_int (helper, &result, &error) != 0)
goto error;
- }
- helper->nsid = sb.st_ino;
- }
+ if (result != 0)
+ {
+ /* ENOSYS indicates that an entire function is unsupported
+ (it's not appropriate for our versions of open/unlink/
+ readlink to sometimes return with ENOSYS depending on how
+ they're called) so we convert ENOSYS to ENOTSUP if setns
+ fails. */
+ if (error == ENOSYS)
+ error = ENOTSUP;
+
+ errno = error;
+ goto error;
+ }
- do_cleanups (old_chain);
+ helper->nsid = sb.st_ino;
+ }
- return MNSH_FS_HELPER;
+ return MNSH_FS_HELPER;
-error:
- saved_errno = errno;
+ error:
+ saved_errno = errno;
- do_cleanups (old_chain);
+ }
errno = saved_errno;
return MNSH_FS_ERROR;
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 7fb7095..63ef2ec 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -115,7 +115,6 @@ procfs_open_1 (struct target_ops *ops, const char *arg, int from_tty)
char buffer[50];
int fd, total_size;
procfs_sysinfo *sysinfo;
- struct cleanup *cleanups;
char nto_procfs_path[PATH_MAX];
/* Offer to kill previous inferiors before opening this target. */
@@ -165,7 +164,7 @@ procfs_open_1 (struct target_ops *ops, const char *arg, int from_tty)
safe_strerror (errno));
error (_("Invalid procfs arg"));
}
- cleanups = make_cleanup_close (fd);
+ gdb::fd_closer close_fd (fd);
sysinfo = (void *) buffer;
if (devctl (fd, DCMD_PROC_SYSINFO, sysinfo, sizeof buffer, 0) != EOK)
@@ -201,7 +200,6 @@ procfs_open_1 (struct target_ops *ops, const char *arg, int from_tty)
}
}
}
- do_cleanups (cleanups);
inf_child_open_target (ops, arg, from_tty);
printf_filtered ("Debugging using %s\n", nto_procfs_path);
@@ -392,7 +390,6 @@ procfs_pidlist (char *args, int from_tty)
do
{
int fd;
- struct cleanup *inner_cleanup;
/* Get the right pid and procfs path for the pid. */
do
@@ -418,7 +415,7 @@ procfs_pidlist (char *args, int from_tty)
buf, errno, safe_strerror (errno));
continue;
}
- inner_cleanup = make_cleanup_close (fd);
+ gdb::fd_closer close_fd (fd);
pidinfo = (procfs_info *) buf;
if (devctl (fd, DCMD_PROC_INFO, pidinfo, sizeof (buf), 0) != EOK)
@@ -451,8 +448,6 @@ procfs_pidlist (char *args, int from_tty)
break;
}
}
-
- do_cleanups (inner_cleanup);
}
while (dirp != NULL);
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 5d940dd..2aaee07 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -31,6 +31,7 @@
#include "regcache.h"
#include "inf-child.h"
#include "filestuff.h"
+#include "common/filestuff.h"
#if defined (NEW_PROC_API)
#define _STRUCTURED_PROC 1 /* Should be done by configure script. */
@@ -883,7 +884,8 @@ load_syscalls (procinfo *pi)
{
error (_("load_syscalls: Can't open /proc/%d/sysent"), pi->pid);
}
- cleanups = make_cleanup_close (sysent_fd);
+
+ gdb::fd_closer close_sysent_fd (sysent_fd);
size = sizeof header - sizeof (prsyscall_t);
if (read (sysent_fd, &header, size) != size)
@@ -899,7 +901,7 @@ load_syscalls (procinfo *pi)
size = header.pr_nsyscalls * sizeof (prsyscall_t);
syscalls = xmalloc (size);
- make_cleanup (free_current_contents, &syscalls);
+ cleanups = make_cleanup (free_current_contents, &syscalls);
if (read (sysent_fd, syscalls, size) != size)
error (_("load_syscalls: Error reading /proc/%d/sysent"), pi->pid);
@@ -2484,7 +2486,6 @@ proc_get_LDT_entry (procinfo *pi, int key)
static struct ssd *ldt_entry = NULL;
#ifdef NEW_PROC_API
char pathname[MAX_PROC_NAME_SIZE];
- struct cleanup *old_chain = NULL;
int fd;
/* Allocate space for one LDT entry.
@@ -2499,8 +2500,8 @@ proc_get_LDT_entry (procinfo *pi, int key)
proc_warn (pi, "proc_get_LDT_entry (open)", __LINE__);
return NULL;
}
- /* Make sure it gets closed again! */
- old_chain = make_cleanup_close (fd);
+
+ gdb::fd_closer close_fd (fd);
/* Now 'read' thru the table, find a match and return it. */
while (read (fd, ldt_entry, sizeof (struct ssd)) == sizeof (struct ssd))
@@ -2512,13 +2513,9 @@ proc_get_LDT_entry (procinfo *pi, int key)
break; /* end of table */
/* If key matches, return this entry. */
if (ldt_entry->sel == key)
- {
- do_cleanups (old_chain);
- return ldt_entry;
- }
+ return ldt_entry;
}
/* Loop ended, match not found. */
- do_cleanups (old_chain);
return NULL;
#else
int nldt, i;
@@ -4912,7 +4909,6 @@ iterate_over_mappings (procinfo *pi, find_memory_region_ftype child_func,
int funcstat;
int map_fd;
int nmap;
- struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
#ifdef NEW_PROC_API
struct stat sbuf;
#endif
@@ -4926,7 +4922,7 @@ iterate_over_mappings (procinfo *pi, find_memory_region_ftype child_func,
proc_error (pi, "iterate_over_mappings (open)", __LINE__);
/* Make sure it gets closed again. */
- make_cleanup_close (map_fd);
+ gdb::fd_closer close_map_fd (map_fd);
/* Use stat to determine the file size, and compute
the number of prmap_t objects it contains. */
@@ -4950,12 +4946,8 @@ iterate_over_mappings (procinfo *pi, find_memory_region_ftype child_func,
for (prmap = prmaps; nmap > 0; prmap++, nmap--)
if ((funcstat = (*func) (prmap, child_func, data)) != 0)
- {
- do_cleanups (cleanups);
- return funcstat;
- }
+ return funcstat;
- do_cleanups (cleanups);
return 0;
}
diff --git a/gdb/source.c b/gdb/source.c
index 4cc862c..3d477ad 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1302,14 +1302,14 @@ get_filename_and_charpos (struct symtab *s, char **fullname)
*fullname = NULL;
return 0;
}
- cleanups = make_cleanup_close (desc);
+
+ gdb::fd_closer close_desc (desc);
if (fullname)
*fullname = s->fullname;
if (s->line_charpos == 0)
linenums_changed = 1;
if (linenums_changed)
find_source_lines (s, desc);
- do_cleanups (cleanups);
return linenums_changed;
}
@@ -1627,7 +1627,6 @@ forward_search_command (char *regex, int from_tty)
int desc;
int line;
char *msg;
- struct cleanup *cleanups;
line = last_line_listed + 1;
@@ -1641,7 +1640,7 @@ forward_search_command (char *regex, int from_tty)
desc = open_source_file (current_source_symtab);
if (desc < 0)
perror_with_name (symtab_to_filename_for_display (current_source_symtab));
- cleanups = make_cleanup_close (desc);
+ gdb::fd_closer close_desc (desc);
if (current_source_symtab->line_charpos == 0)
find_source_lines (current_source_symtab, desc);
@@ -1652,7 +1651,7 @@ forward_search_command (char *regex, int from_tty)
if (lseek (desc, current_source_symtab->line_charpos[line - 1], 0) < 0)
perror_with_name (symtab_to_filename_for_display (current_source_symtab));
- discard_cleanups (cleanups);
+ close_desc.release ();
gdb_file_up stream (fdopen (desc, FDOPEN_MODE));
clearerr (stream.get ());
while (1)
@@ -1726,7 +1725,7 @@ reverse_search_command (char *regex, int from_tty)
desc = open_source_file (current_source_symtab);
if (desc < 0)
perror_with_name (symtab_to_filename_for_display (current_source_symtab));
- cleanups = make_cleanup_close (desc);
+ gdb::fd_closer close_desc (desc);
if (current_source_symtab->line_charpos == 0)
find_source_lines (current_source_symtab, desc);
@@ -1737,7 +1736,7 @@ reverse_search_command (char *regex, int from_tty)
if (lseek (desc, current_source_symtab->line_charpos[line - 1], 0) < 0)
perror_with_name (symtab_to_filename_for_display (current_source_symtab));
- discard_cleanups (cleanups);
+ close_desc.release ();
gdb_file_up stream (fdopen (desc, FDOPEN_MODE));
clearerr (stream.get ());
while (line > 1)
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 09/23] Remove close cleanup
2017-05-03 22:46 ` [RFA 09/23] Remove close cleanup Tom Tromey
@ 2017-06-02 18:08 ` Pedro Alves
2017-07-19 22:52 ` Tom Tromey
0 siblings, 1 reply; 59+ messages in thread
From: Pedro Alves @ 2017-06-02 18:08 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
Patch looks good.
The bit below made me stop for a second:
On 05/03/2017 11:46 PM, Tom Tromey wrote:
> @@ -901,64 +900,59 @@ linux_mntns_access_fs (pid_t pid)
> if (ns == NULL)
> return MNSH_FS_DIRECT;
>
> - old_chain = make_cleanup (null_cleanup, NULL);
> -
> fd = gdb_open_cloexec (linux_ns_filename (ns, pid), O_RDONLY, 0);
> if (fd < 0)
> - goto error;
> -
> - make_cleanup_close (fd);
> + return MNSH_FS_ERROR;
>
> - if (fstat (fd, &sb) != 0)
> - goto error;
> + /* Introduce a new scope here so we can reset errno after
> + closing. */
> + {
> + gdb::fd_closer close_fd (fd);
>
Would the following be too clever?
/* Restore errno on exit to the value saved by the
last save() call. Ctor saves. */
struct scoped_errno_restore
{
scoped_errno_restore () { save (); }
~scoped_errno_restore () { errno = m_value; }
/* Take a snapshot of current errno. */
void save () { m_value = errno; }
private:
int m_value;
};
scoped_errno_restore restore_errno;
gdb::fd_closer close_fd (fd);
/* Save errno and return MNSH_FS_ERROR. */
auto fs_err = [&] ()
{
restore_errno.save ();
return MNSH_FS_ERROR;
};
if (fstat (fd, &sb) != 0)
return fs_err ();
(...)
helper = linux_mntns_get_helper ();
if (helper == NULL)
return fs_err ();
size = mnsh_send_setns (helper, fd, 0);
if (size < 0)
return fs_err ();
[It gets rid of both the scope, and the gotos.]
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 09/23] Remove close cleanup
2017-06-02 18:08 ` Pedro Alves
@ 2017-07-19 22:52 ` Tom Tromey
2017-07-31 19:08 ` Simon Marchi
0 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-07-19 22:52 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> Patch looks good.
Pedro> The bit below made me stop for a second:
[...]
Pedro> Would the following be too clever?
Pedro> /* Restore errno on exit to the value saved by the
Pedro> last save() call. Ctor saves. */
Pedro> struct scoped_errno_restore
[...]
Pedro> /* Save errno and return MNSH_FS_ERROR. */
Pedro> auto fs_err = [&] ()
Pedro> {
Pedro> restore_errno.save ();
Pedro> return MNSH_FS_ERROR;
Pedro> };
[...]
Pedro> [It gets rid of both the scope, and the gotos.]
How about making fd_closer a template, like:
template<int (*CLOSER) (int) = close>
class fd_closer
...
~fd_closer ()
{
if (m_fd != -1)
CLOSER (m_fd);
}
Then in linux-namespaces.c:
/* A function like "close" that saves and restores errno. */
static int
close_saving_fd (int fd)
{
scoped_restore save_errno = make_scoped_restore (&errno);
return close (fd);
}
...
gdb::fd_closer<close_saving_errno> close_fd (fd);
This also removes the scope and the gotos.
The main plus is that it's simpler. The main minus is that it's ad hoc.
Tom
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 09/23] Remove close cleanup
2017-07-19 22:52 ` Tom Tromey
@ 2017-07-31 19:08 ` Simon Marchi
2017-08-01 21:52 ` Tom Tromey
0 siblings, 1 reply; 59+ messages in thread
From: Simon Marchi @ 2017-07-31 19:08 UTC (permalink / raw)
To: Tom Tromey; +Cc: Pedro Alves, gdb-patches
On 2017-07-20 00:27, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> Patch looks good.
> Pedro> The bit below made me stop for a second:
> [...]
>
>
> Pedro> Would the following be too clever?
> Pedro> /* Restore errno on exit to the value saved by the
> Pedro> last save() call. Ctor saves. */
> Pedro> struct scoped_errno_restore
> [...]
>
> Pedro> /* Save errno and return MNSH_FS_ERROR. */
> Pedro> auto fs_err = [&] ()
> Pedro> {
> Pedro> restore_errno.save ();
> Pedro> return MNSH_FS_ERROR;
> Pedro> };
> [...]
>
> Pedro> [It gets rid of both the scope, and the gotos.]
>
> How about making fd_closer a template, like:
>
> template<int (*CLOSER) (int) = close>
> class fd_closer
> ...
> ~fd_closer ()
> {
> if (m_fd != -1)
> CLOSER (m_fd);
> }
>
> Then in linux-namespaces.c:
>
> /* A function like "close" that saves and restores errno. */
>
> static int
> close_saving_fd (int fd)
> {
> scoped_restore save_errno = make_scoped_restore (&errno);
> return close (fd);
> }
>
> ...
>
> gdb::fd_closer<close_saving_errno> close_fd (fd);
>
>
> This also removes the scope and the gotos.
>
> The main plus is that it's simpler. The main minus is that it's ad
> hoc.
>
> Tom
(Responding here although I am reviewing v2)
Instead of finding a solution to preserve errno, I think we should
return it explicitly when needed, it would be more robust and easier to
follow the trail where the value comes from. That would mean changing
static enum mnsh_fs_code
linux_mntns_access_fs (pid_t pid)
to
static enum mnsh_fs_code
linux_mntns_access_fs (pid_t pid, int *errnop)
and possibly others. In that case, *ERRNOP would be set if
MNSH_FS_ERROR is returned.
Simon
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 09/23] Remove close cleanup
2017-07-31 19:08 ` Simon Marchi
@ 2017-08-01 21:52 ` Tom Tromey
0 siblings, 0 replies; 59+ messages in thread
From: Tom Tromey @ 2017-08-01 21:52 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, Pedro Alves, gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
Simon> Instead of finding a solution to preserve errno, I think we should
Simon> return it explicitly when needed, it would be more robust and easier
Simon> to follow the trail where the value comes from. That would mean
Simon> changing
Simon> static enum mnsh_fs_code
Simon> linux_mntns_access_fs (pid_t pid)
Simon> to
Simon> static enum mnsh_fs_code
Simon> linux_mntns_access_fs (pid_t pid, int *errnop)
Simon> and possibly others. In that case, *ERRNOP would be set if
Simon> MNSH_FS_ERROR is returned.
I think somebody else should take this then.
I will see if I can drop this patch from the series.
Tom
^ permalink raw reply [flat|nested] 59+ messages in thread
* [RFA 23/23] Use gdb_argv_up in Python
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
2017-05-03 22:46 ` [RFA 02/23] Introduce and use gdb_file_up Tom Tromey
2017-05-03 22:46 ` [RFA 09/23] Remove close cleanup Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-05 16:29 ` Pedro Alves
2017-05-03 22:46 ` [RFA 10/23] Remove make_cleanup_restore_current_language Tom Tromey
` (20 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes one spot in the Python code to use gdb_argv_up. This
removes the last cleanup from the Python layer.
2017-05-02 Tom Tromey <tom@tromey.com>
* python/py-param.c (compute_enum_values): Use gdb_argv_up.
---
gdb/ChangeLog | 4 ++++
gdb/python/py-param.c | 21 +++++----------------
2 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4dc3bb4..12b9755 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * python/py-param.c (compute_enum_values): Use gdb_argv_up.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* utils.h (struct gdb_argv_deleter): New.
(gdb_argv_up): New typedef.
(gdb_buildargv): Change return type.
diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
index f0d3423..0afed32 100644
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -555,7 +555,6 @@ static int
compute_enum_values (parmpy_object *self, PyObject *enum_values)
{
Py_ssize_t size, i;
- struct cleanup *back_to;
if (! enum_values)
{
@@ -581,36 +580,26 @@ compute_enum_values (parmpy_object *self, PyObject *enum_values)
return 0;
}
- self->enumeration = XCNEWVEC (const char *, size + 1);
- back_to = make_cleanup (free_current_contents, &self->enumeration);
+ gdb_argv_up enumeration (XCNEWVEC (char *, size + 1));
for (i = 0; i < size; ++i)
{
gdbpy_ref<> item (PySequence_GetItem (enum_values, i));
if (item == NULL)
- {
- do_cleanups (back_to);
- return 0;
- }
+ return 0;
if (! gdbpy_is_string (item.get ()))
{
- do_cleanups (back_to);
PyErr_SetString (PyExc_RuntimeError,
_("The enumeration item not a string."));
return 0;
}
- self->enumeration[i]
- = python_string_to_host_string (item.get ()).release ();
+ enumeration[i] = python_string_to_host_string (item.get ()).release ();
if (self->enumeration[i] == NULL)
- {
- do_cleanups (back_to);
- return 0;
- }
- make_cleanup (xfree, (char *) self->enumeration[i]);
+ return 0;
}
- discard_cleanups (back_to);
+ self->enumeration = const_cast<const char**> (enumeration.release ());
return 1;
}
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 23/23] Use gdb_argv_up in Python
2017-05-03 22:46 ` [RFA 23/23] Use gdb_argv_up in Python Tom Tromey
@ 2017-06-05 16:29 ` Pedro Alves
2017-07-19 22:52 ` Tom Tromey
0 siblings, 1 reply; 59+ messages in thread
From: Pedro Alves @ 2017-06-05 16:29 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 05/03/2017 11:46 PM, Tom Tromey wrote:
> This changes one spot in the Python code to use gdb_argv_up. This
> removes the last cleanup from the Python layer.
Hurray! Thanks so much for all this work.
> - self->enumeration[i]
> - = python_string_to_host_string (item.get ()).release ();
> + enumeration[i] = python_string_to_host_string (item.get ()).release ();
> if (self->enumeration[i] == NULL)
I think you need to adjust the if too:
if (enumeration[i] == NULL)
> - {
> - do_cleanups (back_to);
> - return 0;
> - }
> - make_cleanup (xfree, (char *) self->enumeration[i]);
> + return 0;
> }
>
> - discard_cleanups (back_to);
> + self->enumeration = const_cast<const char**> (enumeration.release ());
> return 1;
If I'm reading the code correctly, "self->enumeration" is never
released (by design), right?
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 23/23] Use gdb_argv_up in Python
2017-06-05 16:29 ` Pedro Alves
@ 2017-07-19 22:52 ` Tom Tromey
2017-07-19 22:51 ` Tom Tromey
0 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-07-19 22:52 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>> if (self->enumeration[i] == NULL)
Pedro> I think you need to adjust the if too:
Pedro> if (enumeration[i] == NULL)
Thanks, I made this fix.
>> + self->enumeration = const_cast<const char**> (enumeration.release ());
Pedro> If I'm reading the code correctly, "self->enumeration" is never
Pedro> released (by design), right?
Yes. I think it could be destroyed if the command is deleted, but I
can't remember now if that can happen; and anyway if it can it must be a
weird thing to do.
Tom
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [RFA 23/23] Use gdb_argv_up in Python
2017-07-19 22:52 ` Tom Tromey
@ 2017-07-19 22:51 ` Tom Tromey
0 siblings, 0 replies; 59+ messages in thread
From: Tom Tromey @ 2017-07-19 22:51 UTC (permalink / raw)
To: Tom Tromey; +Cc: Pedro Alves, gdb-patches
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
Tom> Yes. I think it could be destroyed if the command is deleted, but I
Tom> can't remember now if that can happen; and anyway if it can it must be a
Tom> weird thing to do.
... what I mean here is that it currently is never freed, but I think
(not sure) it could be freed in theory when the command is destroyed.
Tom
^ permalink raw reply [flat|nested] 59+ messages in thread
* [RFA 10/23] Remove make_cleanup_restore_current_language
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (2 preceding siblings ...)
2017-05-03 22:46 ` [RFA 23/23] Use gdb_argv_up in Python Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-02 18:18 ` Pedro Alves
2017-05-03 22:46 ` [RFA 03/23] Use gdb_file_up in find_and_open_script Tom Tromey
` (19 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This patch replaces make_cleanup_restore_current_language with an RAII
class that saves the current language, and restores it when the object
is destroyed.
2017-05-02 Tom Tromey <tom@tromey.com>
* utils.h (make_cleanup_restore_current_language): Remove.
* utils.c (do_restore_current_language)
(make_cleanup_restore_current_language): Remove.
* parse.c (parse_exp_in_context_1)
(parse_expression_with_language): Use save_language.
* mi/mi-main.c (mi_cmd_execute): Use save_language.
* language.h (save_language): New class.
---
gdb/ChangeLog | 10 ++++++++++
gdb/language.h | 25 +++++++++++++++++++++++++
gdb/mi/mi-main.c | 6 ++----
gdb/parse.c | 22 +++++++---------------
gdb/utils.c | 22 ----------------------
gdb/utils.h | 2 --
6 files changed, 44 insertions(+), 43 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 50d8794..1c3b738 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,15 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * utils.h (make_cleanup_restore_current_language): Remove.
+ * utils.c (do_restore_current_language)
+ (make_cleanup_restore_current_language): Remove.
+ * parse.c (parse_exp_in_context_1)
+ (parse_expression_with_language): Use save_language.
+ * mi/mi-main.c (mi_cmd_execute): Use save_language.
+ * language.h (save_language): New class.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* source.c (get_filename_and_charpos, forward_search_command)
(reverse_search_command): Use fd_closer.
* procfs.c (load_syscalls, proc_get_LDT_entry)
diff --git a/gdb/language.h b/gdb/language.h
index 58bcbe8..f543da5 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -609,4 +609,29 @@ void default_get_string (struct value *value, gdb_byte **buffer, int *length,
void c_get_string (struct value *value, gdb_byte **buffer, int *length,
struct type **char_type, const char **charset);
+/* Save the current language and restore it upon destruction. */
+
+class save_language
+{
+public:
+
+ explicit save_language (enum language new_lang)
+ : m_lang (current_language->la_language)
+ {
+ set_language (new_lang);
+ }
+
+ ~save_language ()
+ {
+ set_language (m_lang);
+ }
+
+ save_language (const save_language &) = delete;
+ save_language &operator= (const save_language &) = delete;
+
+private:
+
+ enum language m_lang;
+};
+
#endif /* defined (LANGUAGE_H) */
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index cb68fd6..aa44876 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2242,11 +2242,9 @@ mi_cmd_execute (struct mi_parse *parse)
error (_("Invalid frame id: %d"), frame);
}
+ gdb::optional<save_language> lang_saver;
if (parse->language != language_unknown)
- {
- make_cleanup_restore_current_language ();
- set_language (parse->language);
- }
+ lang_saver.emplace (parse->language);
current_context = parse;
diff --git a/gdb/parse.c b/gdb/parse.c
index 3dd7075..84fce19 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -50,6 +50,7 @@
#include "objfiles.h"
#include "user-regs.h"
#include <algorithm>
+#include "common/gdb_optional.h"
/* Standard set of definitions for printing, dumping, prefixifying,
* and evaluating expressions. */
@@ -1136,7 +1137,7 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc,
const struct block *block,
int comma, int void_context_p, int *out_subexp)
{
- struct cleanup *old_chain, *inner_chain;
+ struct cleanup *old_chain;
const struct language_defn *lang = NULL;
struct parser_state ps;
int subexp;
@@ -1214,8 +1215,8 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc,
to the value matching SELECTED_FRAME as set by get_current_arch. */
initialize_expout (&ps, 10, lang, get_current_arch ());
- inner_chain = make_cleanup_restore_current_language ();
- set_language (lang->la_language);
+
+ save_language lang_saver (lang->la_language);
TRY
{
@@ -1250,7 +1251,6 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc,
if (expressiondebug)
dump_prefix_expression (ps.expout, gdb_stdlog);
- do_cleanups (inner_chain);
discard_cleanups (old_chain);
*stringptr = lexptr;
@@ -1275,19 +1275,11 @@ parse_expression (const char *string)
expression_up
parse_expression_with_language (const char *string, enum language lang)
{
- struct cleanup *old_chain = NULL;
-
+ gdb::optional<save_language> lang_saver;
if (current_language->la_language != lang)
- {
- old_chain = make_cleanup_restore_current_language ();
- set_language (lang);
- }
-
- expression_up expr = parse_expression (string);
+ lang_saver.emplace (lang);
- if (old_chain != NULL)
- do_cleanups (old_chain);
- return expr;
+ return parse_expression (string);
}
/* Parse STRING as an expression. If parsing ends in the middle of a
diff --git a/gdb/utils.c b/gdb/utils.c
index bffce78..1a66896 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -307,28 +307,6 @@ make_cleanup_free_so (struct so_list *so)
return make_cleanup (do_free_so, so);
}
-/* Helper for make_cleanup_restore_current_language. */
-
-static void
-do_restore_current_language (void *p)
-{
- enum language saved_lang = (enum language) (uintptr_t) p;
-
- set_language (saved_lang);
-}
-
-/* Remember the current value of CURRENT_LANGUAGE and make it restored when
- the cleanup is run. */
-
-struct cleanup *
-make_cleanup_restore_current_language (void)
-{
- enum language saved_lang = current_language->la_language;
-
- return make_cleanup (do_restore_current_language,
- (void *) (uintptr_t) saved_lang);
-}
-
/* Helper function for make_cleanup_clear_parser_state. */
static void
diff --git a/gdb/utils.h b/gdb/utils.h
index 7afb3c5..1f01bdd 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -91,8 +91,6 @@ extern struct cleanup *make_cleanup_value_free (struct value *);
struct so_list;
extern struct cleanup *make_cleanup_free_so (struct so_list *so);
-extern struct cleanup *make_cleanup_restore_current_language (void);
-
/* A deleter for a hash table. */
struct htab_deleter
{
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 10/23] Remove make_cleanup_restore_current_language
2017-05-03 22:46 ` [RFA 10/23] Remove make_cleanup_restore_current_language Tom Tromey
@ 2017-06-02 18:18 ` Pedro Alves
2017-06-05 13:09 ` Tom Tromey
0 siblings, 1 reply; 59+ messages in thread
From: Pedro Alves @ 2017-06-02 18:18 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
This is OK, but we may we want to take a closer look at
terminology -- we started with scoped_restore, and have added
other types called scoped_restore_whatever meanwhile.
But here we're calling it "save". It seems to me like it may be
better in the long run to pick one naming style for save/restore
types, and stick with it.
Thanks,
Pedro Alves
On 05/03/2017 11:46 PM, Tom Tromey wrote:
> This patch replaces make_cleanup_restore_current_language with an RAII
> class that saves the current language, and restores it when the object
> is destroyed.
>
> 2017-05-02 Tom Tromey <tom@tromey.com>
>
> * utils.h (make_cleanup_restore_current_language): Remove.
> * utils.c (do_restore_current_language)
> (make_cleanup_restore_current_language): Remove.
> * parse.c (parse_exp_in_context_1)
> (parse_expression_with_language): Use save_language.
> * mi/mi-main.c (mi_cmd_execute): Use save_language.
> * language.h (save_language): New class.
> ---
> gdb/ChangeLog | 10 ++++++++++
> gdb/language.h | 25 +++++++++++++++++++++++++
> gdb/mi/mi-main.c | 6 ++----
> gdb/parse.c | 22 +++++++---------------
> gdb/utils.c | 22 ----------------------
> gdb/utils.h | 2 --
> 6 files changed, 44 insertions(+), 43 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 50d8794..1c3b738 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,15 @@
> 2017-05-02 Tom Tromey <tom@tromey.com>
>
> + * utils.h (make_cleanup_restore_current_language): Remove.
> + * utils.c (do_restore_current_language)
> + (make_cleanup_restore_current_language): Remove.
> + * parse.c (parse_exp_in_context_1)
> + (parse_expression_with_language): Use save_language.
> + * mi/mi-main.c (mi_cmd_execute): Use save_language.
> + * language.h (save_language): New class.
> +
> +2017-05-02 Tom Tromey <tom@tromey.com>
> +
> * source.c (get_filename_and_charpos, forward_search_command)
> (reverse_search_command): Use fd_closer.
> * procfs.c (load_syscalls, proc_get_LDT_entry)
> diff --git a/gdb/language.h b/gdb/language.h
> index 58bcbe8..f543da5 100644
> --- a/gdb/language.h
> +++ b/gdb/language.h
> @@ -609,4 +609,29 @@ void default_get_string (struct value *value, gdb_byte **buffer, int *length,
> void c_get_string (struct value *value, gdb_byte **buffer, int *length,
> struct type **char_type, const char **charset);
>
> +/* Save the current language and restore it upon destruction. */
> +
> +class save_language
> +{
> +public:
> +
> + explicit save_language (enum language new_lang)
> + : m_lang (current_language->la_language)
> + {
> + set_language (new_lang);
> + }
> +
> + ~save_language ()
> + {
> + set_language (m_lang);
> + }
> +
> + save_language (const save_language &) = delete;
> + save_language &operator= (const save_language &) = delete;
> +
> +private:
> +
> + enum language m_lang;
> +};
> +
> #endif /* defined (LANGUAGE_H) */
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index cb68fd6..aa44876 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -2242,11 +2242,9 @@ mi_cmd_execute (struct mi_parse *parse)
> error (_("Invalid frame id: %d"), frame);
> }
>
> + gdb::optional<save_language> lang_saver;
> if (parse->language != language_unknown)
> - {
> - make_cleanup_restore_current_language ();
> - set_language (parse->language);
> - }
> + lang_saver.emplace (parse->language);
>
> current_context = parse;
>
> diff --git a/gdb/parse.c b/gdb/parse.c
> index 3dd7075..84fce19 100644
> --- a/gdb/parse.c
> +++ b/gdb/parse.c
> @@ -50,6 +50,7 @@
> #include "objfiles.h"
> #include "user-regs.h"
> #include <algorithm>
> +#include "common/gdb_optional.h"
>
> /* Standard set of definitions for printing, dumping, prefixifying,
> * and evaluating expressions. */
> @@ -1136,7 +1137,7 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc,
> const struct block *block,
> int comma, int void_context_p, int *out_subexp)
> {
> - struct cleanup *old_chain, *inner_chain;
> + struct cleanup *old_chain;
> const struct language_defn *lang = NULL;
> struct parser_state ps;
> int subexp;
> @@ -1214,8 +1215,8 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc,
> to the value matching SELECTED_FRAME as set by get_current_arch. */
>
> initialize_expout (&ps, 10, lang, get_current_arch ());
> - inner_chain = make_cleanup_restore_current_language ();
> - set_language (lang->la_language);
> +
> + save_language lang_saver (lang->la_language);
>
> TRY
> {
> @@ -1250,7 +1251,6 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc,
> if (expressiondebug)
> dump_prefix_expression (ps.expout, gdb_stdlog);
>
> - do_cleanups (inner_chain);
> discard_cleanups (old_chain);
>
> *stringptr = lexptr;
> @@ -1275,19 +1275,11 @@ parse_expression (const char *string)
> expression_up
> parse_expression_with_language (const char *string, enum language lang)
> {
> - struct cleanup *old_chain = NULL;
> -
> + gdb::optional<save_language> lang_saver;
> if (current_language->la_language != lang)
> - {
> - old_chain = make_cleanup_restore_current_language ();
> - set_language (lang);
> - }
> -
> - expression_up expr = parse_expression (string);
> + lang_saver.emplace (lang);
>
> - if (old_chain != NULL)
> - do_cleanups (old_chain);
> - return expr;
> + return parse_expression (string);
> }
>
> /* Parse STRING as an expression. If parsing ends in the middle of a
> diff --git a/gdb/utils.c b/gdb/utils.c
> index bffce78..1a66896 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -307,28 +307,6 @@ make_cleanup_free_so (struct so_list *so)
> return make_cleanup (do_free_so, so);
> }
>
> -/* Helper for make_cleanup_restore_current_language. */
> -
> -static void
> -do_restore_current_language (void *p)
> -{
> - enum language saved_lang = (enum language) (uintptr_t) p;
> -
> - set_language (saved_lang);
> -}
> -
> -/* Remember the current value of CURRENT_LANGUAGE and make it restored when
> - the cleanup is run. */
> -
> -struct cleanup *
> -make_cleanup_restore_current_language (void)
> -{
> - enum language saved_lang = current_language->la_language;
> -
> - return make_cleanup (do_restore_current_language,
> - (void *) (uintptr_t) saved_lang);
> -}
> -
> /* Helper function for make_cleanup_clear_parser_state. */
>
> static void
> diff --git a/gdb/utils.h b/gdb/utils.h
> index 7afb3c5..1f01bdd 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -91,8 +91,6 @@ extern struct cleanup *make_cleanup_value_free (struct value *);
> struct so_list;
> extern struct cleanup *make_cleanup_free_so (struct so_list *so);
>
> -extern struct cleanup *make_cleanup_restore_current_language (void);
> -
> /* A deleter for a hash table. */
> struct htab_deleter
> {
>
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 10/23] Remove make_cleanup_restore_current_language
2017-06-02 18:18 ` Pedro Alves
@ 2017-06-05 13:09 ` Tom Tromey
0 siblings, 0 replies; 59+ messages in thread
From: Tom Tromey @ 2017-06-05 13:09 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> This is OK, but we may we want to take a closer look at
Pedro> terminology -- we started with scoped_restore, and have added
Pedro> other types called scoped_restore_whatever meanwhile.
Pedro> But here we're calling it "save". It seems to me like it may be
Pedro> better in the long run to pick one naming style for save/restore
Pedro> types, and stick with it.
I just went ahead and changed it to scoped_restore_language.
Tom
^ permalink raw reply [flat|nested] 59+ messages in thread
* [RFA 03/23] Use gdb_file_up in find_and_open_script
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (3 preceding siblings ...)
2017-05-03 22:46 ` [RFA 10/23] Remove make_cleanup_restore_current_language Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-02 17:24 ` Pedro Alves
2017-05-03 22:46 ` [RFA 01/23] Introduce and use ui_out_emit_table Tom Tromey
` (18 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes find_and_open_script to return a gdb_file_up. The result
is also used to indicate whether the function succeeded, so the patch
also removes the "streamp" argument; and finally, the type of the
"full_path" argument is changed to remove more cleanups from the
callers.
ChangeLog
2017-05-02 Tom Tromey <tom@tromey.com>
* cli/cli-cmds.c (find_and_open_script): Return gdb_file_p.
Remove "streamp" argument. Change "full_path" to a
unique_xmalloc_ptr.
(source_script_with_search): Update.
* auto-load.c (source_script_file): Update.
* cli/cli-cmds.h (find_and_open_script): Change type.
---
gdb/ChangeLog | 9 +++++++++
gdb/auto-load.c | 32 ++++++++++++++------------------
gdb/cli/cli-cmds.c | 35 +++++++++++++++--------------------
gdb/cli/cli-cmds.h | 7 +++++--
4 files changed, 43 insertions(+), 40 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2bda6ff..76c53be 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * cli/cli-cmds.c (find_and_open_script): Return gdb_file_p.
+ Remove "streamp" argument. Change "full_path" to a
+ unique_xmalloc_ptr.
+ (source_script_with_search): Update.
+ * auto-load.c (source_script_file): Update.
+ * cli/cli-cmds.h (find_and_open_script): Change type.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* xml-support.c (xml_fetch_content_from_file): Update.
* ui-file.c (stdio_file::open): Update.
* tracefile-tfile.c (tfile_start): Update.
diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 292f2ae..dc1bb5d 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -928,10 +928,7 @@ source_script_file (struct auto_load_pspace_info *pspace_info,
const char *section_name, unsigned int offset,
const char *file)
{
- FILE *stream;
- char *full_path;
- int opened, in_hash_table;
- struct cleanup *cleanups;
+ int opened = 0, in_hash_table;
objfile_script_sourcer_func *sourcer;
/* Skip this script if support is not compiled in. */
@@ -953,26 +950,27 @@ source_script_file (struct auto_load_pspace_info *pspace_info,
return;
}
- opened = find_and_open_script (file, 1 /*search_path*/,
- &stream, &full_path);
+ gdb::unique_xmalloc_ptr<char> full_path;
+ gdb_file_up stream = find_and_open_script (file, 1 /*search_path*/,
+ &full_path);
+ const char *path_ptr = full_path.get ();
- cleanups = make_cleanup (null_cleanup, NULL);
- if (opened)
+ if (stream)
{
- make_cleanup_fclose (stream);
- make_cleanup (xfree, full_path);
-
- if (!file_is_auto_load_safe (full_path,
+ if (!file_is_auto_load_safe (full_path.get (),
_("auto-load: Loading %s script "
"\"%s\" from section \"%s\" of "
"objfile \"%s\".\n"),
- ext_lang_name (language), full_path,
+ ext_lang_name (language),
+ full_path.get (),
section_name, objfile_name (objfile)))
opened = 0;
+ else
+ opened = 1;
}
else
{
- full_path = NULL;
+ path_ptr = NULL;
/* If one script isn't found it's not uncommon for more to not be
found either. We don't want to print a message for each script,
@@ -986,14 +984,12 @@ source_script_file (struct auto_load_pspace_info *pspace_info,
section_name, offset);
}
- in_hash_table = maybe_add_script_file (pspace_info, opened, file, full_path,
+ in_hash_table = maybe_add_script_file (pspace_info, opened, file, path_ptr,
language);
/* If this file is not currently loaded, load it. */
if (opened && !in_hash_table)
- sourcer (language, objfile, stream, full_path);
-
- do_cleanups (cleanups);
+ sourcer (language, objfile, stream.get (), path_ptr);
}
/* Subroutine of source_section_scripts to simplify it.
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 2a5b128..c75dd16 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -495,17 +495,16 @@ show_script_ext_mode (struct ui_file *file, int from_tty,
/* Try to open SCRIPT_FILE.
If successful, the full path name is stored in *FULL_PATHP,
- the stream is stored in *STREAMP, and return 1.
- The caller is responsible for freeing *FULL_PATHP.
- If not successful, return 0; errno is set for the last file
+ and the stream is returned.
+ If not successful, return NULL; errno is set for the last file
we tried to open.
If SEARCH_PATH is non-zero, and the file isn't found in cwd,
search for it in the source search path. */
-int
+gdb_file_up
find_and_open_script (const char *script_file, int search_path,
- FILE **streamp, char **full_pathp)
+ gdb::unique_xmalloc_ptr<char> *full_pathp)
{
char *file;
int fd;
@@ -520,8 +519,10 @@ find_and_open_script (const char *script_file, int search_path,
/* Search for and open 'file' on the search path used for source
files. Put the full location in *FULL_PATHP. */
+ char *temp_path;
fd = openp (source_path, search_flags,
- file, O_RDONLY, full_pathp);
+ file, O_RDONLY, &temp_path);
+ full_pathp->reset (temp_path);
if (fd == -1)
{
@@ -533,19 +534,16 @@ find_and_open_script (const char *script_file, int search_path,
do_cleanups (old_cleanups);
- *streamp = fdopen (fd, FOPEN_RT);
- if (*streamp == NULL)
+ FILE *result = fdopen (fd, FOPEN_RT);
+ if (result == NULL)
{
int save_errno = errno;
close (fd);
- if (full_pathp)
- xfree (*full_pathp);
errno = save_errno;
- return 0;
}
- return 1;
+ return gdb_file_up (result);
}
/* Load script FILE, which has already been opened as STREAM.
@@ -596,14 +594,13 @@ source_script_from_stream (FILE *stream, const char *file,
static void
source_script_with_search (const char *file, int from_tty, int search_path)
{
- FILE *stream;
- char *full_path;
- struct cleanup *old_cleanups;
if (file == NULL || *file == 0)
error (_("source command requires file name of file to source."));
- if (!find_and_open_script (file, search_path, &stream, &full_path))
+ gdb::unique_xmalloc_ptr<char> full_path;
+ gdb_file_up stream = find_and_open_script (file, search_path, &full_path);
+ if (!stream)
{
/* The script wasn't found, or was otherwise inaccessible.
If the source command was invoked interactively, throw an
@@ -618,15 +615,13 @@ source_script_with_search (const char *file, int from_tty, int search_path)
}
}
- old_cleanups = make_cleanup (xfree, full_path);
- make_cleanup_fclose (stream);
/* The python support reopens the file, so we need to pass full_path here
in case the file was found on the search path. It's useful to do this
anyway so that error messages show the actual file used. But only do
this if we (may have) used search_path, as printing the full path in
errors for the non-search case can be more noise than signal. */
- source_script_from_stream (stream, file, search_path ? full_path : file);
- do_cleanups (old_cleanups);
+ source_script_from_stream (stream.get (), file,
+ search_path ? full_path.get () : file);
}
/* Wrapper around source_script_with_search to export it to main.c
diff --git a/gdb/cli/cli-cmds.h b/gdb/cli/cli-cmds.h
index 7ff1fca..6e3b3d9 100644
--- a/gdb/cli/cli-cmds.h
+++ b/gdb/cli/cli-cmds.h
@@ -17,6 +17,8 @@
#if !defined (CLI_CMDS_H)
#define CLI_CMDS_H 1
+#include "common/filestuff.h"
+
/* Chain containing all defined commands. */
extern struct cmd_list_element *cmdlist;
@@ -117,8 +119,9 @@ extern void source_script (const char *, int);
/* Exported to objfiles.c. */
-extern int find_and_open_script (const char *file, int search_path,
- FILE **streamp, char **full_path);
+extern gdb_file_up
+ find_and_open_script (const char *file, int search_path,
+ gdb::unique_xmalloc_ptr<char> *full_path);
/* Command tracing state. */
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 03/23] Use gdb_file_up in find_and_open_script
2017-05-03 22:46 ` [RFA 03/23] Use gdb_file_up in find_and_open_script Tom Tromey
@ 2017-06-02 17:24 ` Pedro Alves
2017-07-23 16:08 ` Tom Tromey
0 siblings, 1 reply; 59+ messages in thread
From: Pedro Alves @ 2017-06-02 17:24 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 05/03/2017 11:46 PM, Tom Tromey wrote:
> This changes find_and_open_script to return a gdb_file_up. The result
> is also used to indicate whether the function succeeded, so the patch
> also removes the "streamp" argument; and finally, the type of the
> "full_path" argument is changed to remove more cleanups from the
> callers.
>
This is OK.
> -extern int find_and_open_script (const char *file, int search_path,
> - FILE **streamp, char **full_path);
> +extern gdb_file_up
> + find_and_open_script (const char *file, int search_path,
> + gdb::unique_xmalloc_ptr<char> *full_path);
>
Though I wonder whether using gdb::optional wouldn't look clearer:
/* The script that was opened. */
struct open_script
{
gdb_file_up stream;
gdb::unique_xmalloc_ptr<char> full_path;
};
gdb::optional<open_script>
find_and_open_script (const char *file, int search_path);
open_script opened = find_and_open_script (file, 1 /*search_path*/);
if (opened)
{
if (!file_is_auto_load_safe (res->full_path.get (),
_("auto-load: Loading %s script "
"\"%s\" from section \"%s\" of "
"objfile \"%s\".\n"),
ext_lang_name (language),
opened->full_path.get (),
section_name, objfile_name (objfile)))
opened.reset ();
}
...
/* If this file is not currently loaded, load it. */
if (opened && !in_hash_table)
sourcer (language, objfile, opened->stream, opened->full_path.get ());
}
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 03/23] Use gdb_file_up in find_and_open_script
2017-06-02 17:24 ` Pedro Alves
@ 2017-07-23 16:08 ` Tom Tromey
0 siblings, 0 replies; 59+ messages in thread
From: Tom Tromey @ 2017-07-23 16:08 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>> -extern int find_and_open_script (const char *file, int search_path,
>> - FILE **streamp, char **full_path);
>> +extern gdb_file_up
>> + find_and_open_script (const char *file, int search_path,
>> + gdb::unique_xmalloc_ptr<char> *full_path);
>>
Pedro> Though I wonder whether using gdb::optional wouldn't look clearer:
Pedro> /* The script that was opened. */
Pedro> struct open_script
Pedro> {
Pedro> gdb_file_up stream;
Pedro> gdb::unique_xmalloc_ptr<char> full_path;
Pedro> };
[...]
I went ahead and did it this way.
Tom
^ permalink raw reply [flat|nested] 59+ messages in thread
* [RFA 01/23] Introduce and use ui_out_emit_table
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (4 preceding siblings ...)
2017-05-03 22:46 ` [RFA 03/23] Use gdb_file_up in find_and_open_script Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-02 17:04 ` Pedro Alves
2017-05-03 22:46 ` [RFA 20/23] Avoid some manual memory management in Python Tom Tromey
` (17 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This introduces ui_out_emit_table, similar to the other existing
ui_out RAII classes, and then uses it in a number of places. This
replaces some cleanups.
ChangeLog
2017-05-02 Tom Tromey <tom@tromey.com>
* tracepoint.c (tvariables_info_1): Use ui_out_emit_table.
(info_static_tracepoint_markers_command): Likewise.
* solib.c (info_sharedlibrary_command): Use ui_out_emit_table.
* skip.c (skip_info): Use ui_out_emit_table.
* progspace.c (print_program_space): Use ui_out_emit_table.
* osdata.c (info_osdata): Use ui_out_emit_table.
* mi/mi-cmd-info.c (mi_cmd_info_ada_exceptions): Use
ui_out_emit_table.
* linux-thread-db.c (info_auto_load_libthread_db): Use
ui_out_emit_table.
* inferior.c (print_inferior): Use ui_out_emit_table.
* gdb_bfd.c (maintenance_info_bfds): Use ui_out_emit_table.
* breakpoint.c (breakpoint_1): Use ui_out_emit_table.
* auto-load.c (auto_load_info_scripts): Use ui_out_emit_table.
* ada-tasks.c (print_ada_task_info): Use ui_out_emit_table.
* ui-out.h (class ui_out_emit_table): New.
---
gdb/ChangeLog | 19 ++++++++++++
gdb/ada-tasks.c | 6 +---
gdb/auto-load.c | 18 ++++++------
gdb/breakpoint.c | 81 +++++++++++++++++++++++----------------------------
gdb/gdb_bfd.c | 5 +---
gdb/inferior.c | 6 +---
gdb/linux-thread-db.c | 68 ++++++++++++++++++++++--------------------
gdb/mi/mi-cmd-info.c | 5 ++--
gdb/osdata.c | 3 +-
gdb/progspace.c | 5 +---
gdb/skip.c | 8 ++---
gdb/solib.c | 27 ++++++++---------
gdb/tracepoint.c | 13 +++------
gdb/ui-out.h | 27 +++++++++++++++++
14 files changed, 154 insertions(+), 137 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4e278f5..843aef3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,22 @@
+2017-05-02 Tom Tromey <tom@tromey.com>
+
+ * tracepoint.c (tvariables_info_1): Use ui_out_emit_table.
+ (info_static_tracepoint_markers_command): Likewise.
+ * solib.c (info_sharedlibrary_command): Use ui_out_emit_table.
+ * skip.c (skip_info): Use ui_out_emit_table.
+ * progspace.c (print_program_space): Use ui_out_emit_table.
+ * osdata.c (info_osdata): Use ui_out_emit_table.
+ * mi/mi-cmd-info.c (mi_cmd_info_ada_exceptions): Use
+ ui_out_emit_table.
+ * linux-thread-db.c (info_auto_load_libthread_db): Use
+ ui_out_emit_table.
+ * inferior.c (print_inferior): Use ui_out_emit_table.
+ * gdb_bfd.c (maintenance_info_bfds): Use ui_out_emit_table.
+ * breakpoint.c (breakpoint_1): Use ui_out_emit_table.
+ * auto-load.c (auto_load_info_scripts): Use ui_out_emit_table.
+ * ada-tasks.c (print_ada_task_info): Use ui_out_emit_table.
+ * ui-out.h (class ui_out_emit_table): New.
+
2017-05-03 Alan Hayward <alan.hayward@arm.com>
* frv-linux-tdep.c (frv_linux_supply_gregset): Use raw_supply_zeroed.
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index ae43da5..cf43f0e 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -1008,7 +1008,6 @@ print_ada_task_info (struct ui_out *uiout,
struct ada_tasks_inferior_data *data;
int taskno, nb_tasks;
int taskno_arg = 0;
- struct cleanup *old_chain;
int nb_columns;
if (ada_build_task_list () == 0)
@@ -1047,8 +1046,7 @@ print_ada_task_info (struct ui_out *uiout,
nb_tasks = VEC_length (ada_task_info_s, data->task_list);
nb_columns = uiout->is_mi_like_p () ? 8 : 7;
- old_chain = make_cleanup_ui_out_table_begin_end (uiout, nb_columns,
- nb_tasks, "tasks");
+ ui_out_emit_table table_emitter (uiout, nb_columns, nb_tasks, "tasks");
uiout->table_header (1, ui_left, "current", "");
uiout->table_header (3, ui_right, "id", "ID");
uiout->table_header (9, ui_right, "task-id", "TID");
@@ -1143,8 +1141,6 @@ print_ada_task_info (struct ui_out *uiout,
uiout->text ("\n");
}
-
- do_cleanups (old_chain);
}
/* Print a detailed description of the Ada task whose ID is TASKNO_STR
diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 37bf942..75ce828 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -1382,18 +1382,18 @@ auto_load_info_scripts (char *pattern, int from_tty,
if (nr_scripts > 0 && pattern == auto_load_info_scripts_pattern_nl)
uiout->text ("\n");
- /* Note: This creates a cleanup to output the table end marker. */
- make_cleanup_ui_out_table_begin_end (uiout, 2, nr_scripts,
- "AutoLoadedScriptsTable");
+ {
+ ui_out_emit_table table_emitter (uiout, 2, nr_scripts,
+ "AutoLoadedScriptsTable");
- uiout->table_header (7, ui_left, "loaded", "Loaded");
- uiout->table_header (70, ui_left, "script", "Script");
- uiout->table_body ();
+ uiout->table_header (7, ui_left, "loaded", "Loaded");
+ uiout->table_header (70, ui_left, "script", "Script");
+ uiout->table_body ();
- print_scripts (script_files);
- print_scripts (script_texts);
+ print_scripts (script_files);
+ print_scripts (script_texts);
+ }
- /* Finish up the table before checking for no matching scripts. */
do_cleanups (script_chain);
if (nr_scripts == 0)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index e8d8d09..2bc7b80 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6794,7 +6794,6 @@ breakpoint_1 (char *args, int allflag,
struct breakpoint *b;
struct bp_location *last_loc = NULL;
int nr_printable_breakpoints;
- struct cleanup *bkpttbl_chain;
struct value_print_options opts;
int print_address_bits = 0;
int print_type_col_width = 14;
@@ -6837,48 +6836,43 @@ breakpoint_1 (char *args, int allflag,
}
}
- if (opts.addressprint)
- bkpttbl_chain
- = make_cleanup_ui_out_table_begin_end (uiout, 6,
- nr_printable_breakpoints,
- "BreakpointTable");
- else
- bkpttbl_chain
- = make_cleanup_ui_out_table_begin_end (uiout, 5,
- nr_printable_breakpoints,
- "BreakpointTable");
-
- if (nr_printable_breakpoints > 0)
- annotate_breakpoints_headers ();
- if (nr_printable_breakpoints > 0)
- annotate_field (0);
- uiout->table_header (7, ui_left, "number", "Num"); /* 1 */
- if (nr_printable_breakpoints > 0)
- annotate_field (1);
- uiout->table_header (print_type_col_width, ui_left, "type", "Type"); /* 2 */
- if (nr_printable_breakpoints > 0)
- annotate_field (2);
- uiout->table_header (4, ui_left, "disp", "Disp"); /* 3 */
- if (nr_printable_breakpoints > 0)
- annotate_field (3);
- uiout->table_header (3, ui_left, "enabled", "Enb"); /* 4 */
- if (opts.addressprint)
- {
- if (nr_printable_breakpoints > 0)
- annotate_field (4);
- if (print_address_bits <= 32)
- uiout->table_header (10, ui_left, "addr", "Address"); /* 5 */
- else
- uiout->table_header (18, ui_left, "addr", "Address"); /* 5 */
- }
- if (nr_printable_breakpoints > 0)
- annotate_field (5);
- uiout->table_header (40, ui_noalign, "what", "What"); /* 6 */
- uiout->table_body ();
- if (nr_printable_breakpoints > 0)
- annotate_breakpoints_table ();
+ {
+ ui_out_emit_table table_emitter (uiout,
+ opts.addressprint ? 6 : 5,
+ nr_printable_breakpoints,
+ "BreakpointTable");
+
+ if (nr_printable_breakpoints > 0)
+ annotate_breakpoints_headers ();
+ if (nr_printable_breakpoints > 0)
+ annotate_field (0);
+ uiout->table_header (7, ui_left, "number", "Num"); /* 1 */
+ if (nr_printable_breakpoints > 0)
+ annotate_field (1);
+ uiout->table_header (print_type_col_width, ui_left, "type", "Type"); /* 2 */
+ if (nr_printable_breakpoints > 0)
+ annotate_field (2);
+ uiout->table_header (4, ui_left, "disp", "Disp"); /* 3 */
+ if (nr_printable_breakpoints > 0)
+ annotate_field (3);
+ uiout->table_header (3, ui_left, "enabled", "Enb"); /* 4 */
+ if (opts.addressprint)
+ {
+ if (nr_printable_breakpoints > 0)
+ annotate_field (4);
+ if (print_address_bits <= 32)
+ uiout->table_header (10, ui_left, "addr", "Address"); /* 5 */
+ else
+ uiout->table_header (18, ui_left, "addr", "Address"); /* 5 */
+ }
+ if (nr_printable_breakpoints > 0)
+ annotate_field (5);
+ uiout->table_header (40, ui_noalign, "what", "What"); /* 6 */
+ uiout->table_body ();
+ if (nr_printable_breakpoints > 0)
+ annotate_breakpoints_table ();
- ALL_BREAKPOINTS (b)
+ ALL_BREAKPOINTS (b)
{
QUIT;
/* If we have a filter, only list the breakpoints it accepts. */
@@ -6906,8 +6900,7 @@ breakpoint_1 (char *args, int allflag,
if (allflag || user_breakpoint_p (b))
print_one_breakpoint (b, &last_loc, allflag);
}
-
- do_cleanups (bkpttbl_chain);
+ }
if (nr_printable_breakpoints == 0)
{
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 2230127..c0dc191 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -976,18 +976,15 @@ print_one_bfd (void **slot, void *data)
static void
maintenance_info_bfds (char *arg, int from_tty)
{
- struct cleanup *cleanup;
struct ui_out *uiout = current_uiout;
- cleanup = make_cleanup_ui_out_table_begin_end (uiout, 3, -1, "bfds");
+ ui_out_emit_table table_emitter (uiout, 3, -1, "bfds");
uiout->table_header (10, ui_left, "refcount", "Refcount");
uiout->table_header (18, ui_left, "addr", "Address");
uiout->table_header (40, ui_left, "filename", "Filename");
uiout->table_body ();
htab_traverse (all_bfds, print_one_bfd, uiout);
-
- do_cleanups (cleanup);
}
/* -Wmissing-prototypes */
diff --git a/gdb/inferior.c b/gdb/inferior.c
index db23df9..002296f 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -559,7 +559,6 @@ static void
print_inferior (struct ui_out *uiout, char *requested_inferiors)
{
struct inferior *inf;
- struct cleanup *old_chain;
int inf_count = 0;
/* Compute number of inferiors we will print. */
@@ -577,8 +576,7 @@ print_inferior (struct ui_out *uiout, char *requested_inferiors)
return;
}
- old_chain = make_cleanup_ui_out_table_begin_end (uiout, 4, inf_count,
- "inferiors");
+ ui_out_emit_table table_emitter (uiout, 4, inf_count, "inferiors");
uiout->table_header (1, ui_left, "current", "");
uiout->table_header (4, ui_left, "number", "Num");
uiout->table_header (17, ui_left, "target-id", "Description");
@@ -622,8 +620,6 @@ print_inferior (struct ui_out *uiout, char *requested_inferiors)
uiout->text ("\n");
}
-
- do_cleanups (old_chain);
}
static void
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 86254f8..cf68013 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -1623,45 +1623,49 @@ info_auto_load_libthread_db (char *args, int from_tty)
if (info_count > 0 && args == auto_load_info_scripts_pattern_nl)
uiout->text ("\n");
- make_cleanup_ui_out_table_begin_end (uiout, 2, unique_filenames,
- "LinuxThreadDbTable");
+ {
+ ui_out_emit_table table_emitter (uiout, 2, unique_filenames,
+ "LinuxThreadDbTable");
- uiout->table_header (max_filename_len, ui_left, "filename", "Filename");
- uiout->table_header (pids_len, ui_left, "PIDs", "Pids");
- uiout->table_body ();
+ uiout->table_header (max_filename_len, ui_left, "filename", "Filename");
+ uiout->table_header (pids_len, ui_left, "PIDs", "Pids");
+ uiout->table_body ();
- pids = (char *) xmalloc (max_pids_len + 1);
- make_cleanup (xfree, pids);
+ pids = (char *) xmalloc (max_pids_len + 1);
+ make_cleanup (xfree, pids);
- /* Note I is incremented inside the cycle, not at its end. */
- for (i = 0; i < info_count;)
- {
- ui_out_emit_tuple tuple_emitter (uiout, NULL);
- char *pids_end;
-
- info = array[i];
- uiout->field_string ("filename", info->filename);
- pids_end = pids;
+ /* Note I is incremented inside the cycle, not at its end. */
+ for (i = 0; i < info_count;)
+ {
+ ui_out_emit_tuple tuple_emitter (uiout, NULL);
+ char *pids_end;
- while (i < info_count && strcmp (info->filename, array[i]->filename) == 0)
- {
- if (pids_end != pids)
- {
- *pids_end++ = ',';
- *pids_end++ = ' ';
- }
- pids_end += xsnprintf (pids_end, &pids[max_pids_len + 1] - pids_end,
- "%u", array[i]->pid);
- gdb_assert (pids_end < &pids[max_pids_len + 1]);
+ info = array[i];
+ uiout->field_string ("filename", info->filename);
+ pids_end = pids;
- i++;
- }
- *pids_end = '\0';
+ while (i < info_count && strcmp (info->filename,
+ array[i]->filename) == 0)
+ {
+ if (pids_end != pids)
+ {
+ *pids_end++ = ',';
+ *pids_end++ = ' ';
+ }
+ pids_end += xsnprintf (pids_end,
+ &pids[max_pids_len + 1] - pids_end,
+ "%u", array[i]->pid);
+ gdb_assert (pids_end < &pids[max_pids_len + 1]);
+
+ i++;
+ }
+ *pids_end = '\0';
- uiout->field_string ("pids", pids);
+ uiout->field_string ("pids", pids);
- uiout->text ("\n");
- }
+ uiout->text ("\n");
+ }
+ }
do_cleanups (back_to);
diff --git a/gdb/mi/mi-cmd-info.c b/gdb/mi/mi-cmd-info.c
index 7da4f90..fa0d16e 100644
--- a/gdb/mi/mi-cmd-info.c
+++ b/gdb/mi/mi-cmd-info.c
@@ -51,8 +51,9 @@ mi_cmd_info_ada_exceptions (const char *command, char **argv, int argc)
exceptions = ada_exceptions_list (regexp);
old_chain = make_cleanup (VEC_cleanup (ada_exc_info), &exceptions);
- make_cleanup_ui_out_table_begin_end
- (uiout, 2, VEC_length (ada_exc_info, exceptions), "ada-exceptions");
+ ui_out_emit_table table_emitter (uiout, 2,
+ VEC_length (ada_exc_info, exceptions),
+ "ada-exceptions");
uiout->table_header (1, ui_left, "name", "Name");
uiout->table_header (1, ui_left, "address", "Address");
uiout->table_body ();
diff --git a/gdb/osdata.c b/gdb/osdata.c
index b9014dd..5458e2f 100644
--- a/gdb/osdata.c
+++ b/gdb/osdata.c
@@ -337,8 +337,7 @@ info_osdata (const char *type)
}
}
- make_cleanup_ui_out_table_begin_end (uiout, ncols, nrows,
- "OSDataTable");
+ ui_out_emit_table table_emitter (uiout, ncols, nrows, "OSDataTable");
/* With no columns/items, we just output an empty table, but we
still output the table. This matters for MI. */
diff --git a/gdb/progspace.c b/gdb/progspace.c
index b37701e..ff9f28d 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -288,7 +288,6 @@ print_program_space (struct ui_out *uiout, int requested)
{
struct program_space *pspace;
int count = 0;
- struct cleanup *old_chain;
/* Compute number of pspaces we will print. */
ALL_PSPACES (pspace)
@@ -302,7 +301,7 @@ print_program_space (struct ui_out *uiout, int requested)
/* There should always be at least one. */
gdb_assert (count > 0);
- old_chain = make_cleanup_ui_out_table_begin_end (uiout, 3, count, "pspaces");
+ ui_out_emit_table table_emitter (uiout, 3, count, "pspaces");
uiout->table_header (1, ui_left, "current", "");
uiout->table_header (4, ui_left, "id", "Id");
uiout->table_header (17, ui_left, "exec", "Executable");
@@ -354,8 +353,6 @@ print_program_space (struct ui_out *uiout, int requested)
uiout->text ("\n");
}
-
- do_cleanups (old_chain);
}
/* Boolean test for an already-known program space id. */
diff --git a/gdb/skip.c b/gdb/skip.c
index 4bd8a9e..33ed8d8 100644
--- a/gdb/skip.c
+++ b/gdb/skip.c
@@ -360,7 +360,6 @@ skip_info (char *arg, int from_tty)
struct skiplist_entry *e;
int num_printable_entries = 0;
struct value_print_options opts;
- struct cleanup *tbl_chain;
get_user_print_options (&opts);
@@ -381,9 +380,8 @@ skip_info (char *arg, int from_tty)
return;
}
- tbl_chain = make_cleanup_ui_out_table_begin_end (current_uiout, 6,
- num_printable_entries,
- "SkiplistTable");
+ ui_out_emit_table table_emitter (current_uiout, 6, num_printable_entries,
+ "SkiplistTable");
current_uiout->table_header (5, ui_left, "number", "Num"); /* 1 */
current_uiout->table_header (3, ui_left, "enabled", "Enb"); /* 2 */
@@ -425,8 +423,6 @@ skip_info (char *arg, int from_tty)
current_uiout->text ("\n");
}
-
- do_cleanups (tbl_chain);
}
static void
diff --git a/gdb/solib.c b/gdb/solib.c
index 491c18a..a905193 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -1046,7 +1046,6 @@ info_sharedlibrary_command (char *pattern, int from_tty)
int so_missing_debug_info = 0;
int addr_width;
int nr_libs;
- struct cleanup *table_cleanup;
struct gdbarch *gdbarch = target_gdbarch ();
struct ui_out *uiout = current_uiout;
@@ -1063,8 +1062,8 @@ info_sharedlibrary_command (char *pattern, int from_tty)
update_solib_list (from_tty);
- /* make_cleanup_ui_out_table_begin_end needs to know the number of
- rows, so we need to make two passes over the libs. */
+ /* ui_out_emit_table table_emitter needs to know the number of rows,
+ so we need to make two passes over the libs. */
for (nr_libs = 0, so = so_list_head; so; so = so->next)
{
@@ -1076,19 +1075,18 @@ info_sharedlibrary_command (char *pattern, int from_tty)
}
}
- table_cleanup =
- make_cleanup_ui_out_table_begin_end (uiout, 4, nr_libs,
- "SharedLibraryTable");
+ {
+ ui_out_emit_table table_emitter (uiout, 4, nr_libs, "SharedLibraryTable");
- /* The "- 1" is because ui_out adds one space between columns. */
- uiout->table_header (addr_width - 1, ui_left, "from", "From");
- uiout->table_header (addr_width - 1, ui_left, "to", "To");
- uiout->table_header (12 - 1, ui_left, "syms-read", "Syms Read");
- uiout->table_header (0, ui_noalign, "name", "Shared Object Library");
+ /* The "- 1" is because ui_out adds one space between columns. */
+ uiout->table_header (addr_width - 1, ui_left, "from", "From");
+ uiout->table_header (addr_width - 1, ui_left, "to", "To");
+ uiout->table_header (12 - 1, ui_left, "syms-read", "Syms Read");
+ uiout->table_header (0, ui_noalign, "name", "Shared Object Library");
- uiout->table_body ();
+ uiout->table_body ();
- ALL_SO_LIBS (so)
+ ALL_SO_LIBS (so)
{
if (! so->so_name[0])
continue;
@@ -1122,8 +1120,7 @@ info_sharedlibrary_command (char *pattern, int from_tty)
uiout->text ("\n");
}
-
- do_cleanups (table_cleanup);
+ }
if (nr_libs == 0)
{
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 808afde..3a39ebc 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -482,7 +482,6 @@ tvariables_info_1 (void)
struct trace_state_variable *tsv;
int ix;
int count = 0;
- struct cleanup *back_to;
struct ui_out *uiout = current_uiout;
if (VEC_length (tsv_s, tvariables) == 0 && !uiout->is_mi_like_p ())
@@ -496,8 +495,7 @@ tvariables_info_1 (void)
tsv->value_known = target_get_trace_state_variable_value (tsv->number,
&(tsv->value));
- back_to = make_cleanup_ui_out_table_begin_end (uiout, 3,
- count, "trace-variables");
+ ui_out_emit_table table_emitter (uiout, 3, count, "trace-variables");
uiout->table_header (15, ui_left, "name", "Name");
uiout->table_header (11, ui_left, "initial", "Initial");
uiout->table_header (11, ui_left, "current", "Current");
@@ -531,8 +529,6 @@ tvariables_info_1 (void)
uiout->field_string ("current", c);
uiout->text ("\n");
}
-
- do_cleanups (back_to);
}
/* List all the trace state variables. */
@@ -3954,9 +3950,8 @@ info_static_tracepoint_markers_command (char *arg, int from_tty)
don't work without in-process agent, so we don't bother users to type
`set agent on' when to use static tracepoint. */
- old_chain
- = make_cleanup_ui_out_table_begin_end (uiout, 5, -1,
- "StaticTracepointMarkersTable");
+ ui_out_emit_table table_emitter (uiout, 5, -1,
+ "StaticTracepointMarkersTable");
uiout->table_header (7, ui_left, "counter", "Cnt");
@@ -3972,7 +3967,7 @@ info_static_tracepoint_markers_command (char *arg, int from_tty)
uiout->table_body ();
markers = target_static_tracepoint_markers_by_strid (NULL);
- make_cleanup (VEC_cleanup (static_tracepoint_marker_p), &markers);
+ old_chain = make_cleanup (VEC_cleanup (static_tracepoint_marker_p), &markers);
for (i = 0;
VEC_iterate (static_tracepoint_marker_p,
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index 9278cab..2293f26 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -220,4 +220,31 @@ private:
typedef ui_out_emit_type<ui_out_type_tuple> ui_out_emit_tuple;
typedef ui_out_emit_type<ui_out_type_list> ui_out_emit_list;
+/* This is similar to make_cleanup_ui_out_table_begin_end, but written
+ as an RAII class. */
+class ui_out_emit_table
+{
+public:
+
+ ui_out_emit_table (struct ui_out *uiout, int nr_cols, int nr_rows,
+ const char *tblid)
+
+ : m_uiout (uiout)
+ {
+ m_uiout->table_begin (nr_cols, nr_rows, tblid);
+ }
+
+ ~ui_out_emit_table ()
+ {
+ m_uiout->table_end ();
+ }
+
+ ui_out_emit_table (const ui_out_emit_table &) = delete;
+ ui_out_emit_table &operator= (const ui_out_emit_table &) = delete;
+
+private:
+
+ struct ui_out *m_uiout;
+};
+
#endif /* UI_OUT_H */
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* [RFA 20/23] Avoid some manual memory management in Python
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (5 preceding siblings ...)
2017-05-03 22:46 ` [RFA 01/23] Introduce and use ui_out_emit_table Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-05 13:55 ` Pedro Alves
2017-05-03 22:46 ` [RFA 12/23] More uses of scoped_restore Tom Tromey
` (16 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes a few places in the Python code to avoid manual memory
management, in favor of letting std::string do the work.
2017-05-02 Tom Tromey <tom@tromey.com>
* python/python.c (compute_python_string): Return std::string.
(gdbpy_eval_from_control_command): Update.
(do_start_initialization): Use std::string.
* python/py-varobj.c (py_varobj_iter_next): Use string_printf, not
xstrprintf.
* python/py-breakpoint.c (local_setattro): Use string_printf, not
xstrprintf.
---
gdb/ChangeLog | 10 ++++++++++
gdb/python/py-breakpoint.c | 15 ++++++---------
gdb/python/py-varobj.c | 9 ++++-----
gdb/python/python.c | 37 +++++++++----------------------------
4 files changed, 29 insertions(+), 42 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5e37c48..fc88456 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,15 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * python/python.c (compute_python_string): Return std::string.
+ (gdbpy_eval_from_control_command): Update.
+ (do_start_initialization): Use std::string.
+ * python/py-varobj.c (py_varobj_iter_next): Use string_printf, not
+ xstrprintf.
+ * python/py-breakpoint.c (local_setattro): Use string_printf, not
+ xstrprintf.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* top.h (do_restore_instream_cleanup): Remove.
* top.c (do_restore_instream_cleanup): Remove.
(read_command_file): Use scoped_restore.
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 64de803..6156eb6 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -1026,15 +1026,12 @@ local_setattro (PyObject *self, PyObject *name, PyObject *v)
extlang = get_breakpoint_cond_ext_lang (obj->bp, EXT_LANG_PYTHON);
if (extlang != NULL)
{
- char *error_text;
-
- error_text
- = xstrprintf (_("Only one stop condition allowed. There is"
- " currently a %s stop condition defined for"
- " this breakpoint."),
- ext_lang_capitalized_name (extlang));
- PyErr_SetString (PyExc_RuntimeError, error_text);
- xfree (error_text);
+ std::string error_text
+ = string_printf (_("Only one stop condition allowed. There is"
+ " currently a %s stop condition defined for"
+ " this breakpoint."),
+ ext_lang_capitalized_name (extlang));
+ PyErr_SetString (PyExc_RuntimeError, error_text.c_str ());
return -1;
}
}
diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
index e858556..5f6ab64 100644
--- a/gdb/python/py-varobj.c
+++ b/gdb/python/py-varobj.c
@@ -71,7 +71,6 @@ py_varobj_iter_next (struct varobj_iter *self)
if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
{
PyObject *type, *value, *trace;
- char *name_str;
PyErr_Fetch (&type, &value, &trace);
gdb::unique_xmalloc_ptr<char>
@@ -85,10 +84,10 @@ py_varobj_iter_next (struct varobj_iter *self)
return NULL;
}
- name_str = xstrprintf ("<error at %d>",
- self->next_raw_index++);
- item.reset (Py_BuildValue ("(ss)", name_str, value_str.get ()));
- xfree (name_str);
+ std::string name_str = string_printf ("<error at %d>",
+ self->next_raw_index++);
+ item.reset (Py_BuildValue ("(ss)", name_str.c_str (),
+ value_str.get ()));
if (item == NULL)
{
gdbpy_print_stack ();
diff --git a/gdb/python/python.c b/gdb/python/python.c
index be92f36..17ab04e 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -365,32 +365,19 @@ python_run_simple_file (FILE *file, const char *filename)
}
/* Given a command_line, return a command string suitable for passing
- to Python. Lines in the string are separated by newlines. The
- return value is allocated using xmalloc and the caller is
- responsible for freeing it. */
+ to Python. Lines in the string are separated by newlines. */
-static char *
+static std::string
compute_python_string (struct command_line *l)
{
struct command_line *iter;
- char *script = NULL;
- int size = 0;
- int here;
+ std::string script;
for (iter = l; iter; iter = iter->next)
- size += strlen (iter->line) + 1;
-
- script = (char *) xmalloc (size + 1);
- here = 0;
- for (iter = l; iter; iter = iter->next)
{
- int len = strlen (iter->line);
-
- strcpy (&script[here], iter->line);
- here += len;
- script[here++] = '\n';
+ script += iter->line;
+ script += "\n";
}
- script[here] = '\0';
return script;
}
@@ -402,16 +389,14 @@ gdbpy_eval_from_control_command (const struct extension_language_defn *extlang,
struct command_line *cmd)
{
int ret;
- char *script;
if (cmd->body_count != 1)
error (_("Invalid \"python\" block structure."));
gdbpy_enter enter_py (get_current_arch (), current_language);
- script = compute_python_string (cmd->body_list[0]);
- ret = PyRun_SimpleString (script);
- xfree (script);
+ std::string script = compute_python_string (cmd->body_list[0]);
+ ret = PyRun_SimpleString (script.c_str ());
if (ret)
error (_("Error while executing Python code."));
}
@@ -1532,7 +1517,6 @@ do_start_initialization ()
#ifdef IS_PY3K
int i;
size_t progsize, count;
- char *oldloc;
wchar_t *progname_copy;
#endif
@@ -1546,25 +1530,22 @@ do_start_initialization ()
progname = concat (ldirname (python_libdir).c_str (), SLASH_STRING, "bin",
SLASH_STRING, "python", (char *) NULL);
#ifdef IS_PY3K
- oldloc = xstrdup (setlocale (LC_ALL, NULL));
+ std::string oldloc = setlocale (LC_ALL, NULL);
setlocale (LC_ALL, "");
progsize = strlen (progname);
progname_copy = (wchar_t *) PyMem_Malloc ((progsize + 1) * sizeof (wchar_t));
if (!progname_copy)
{
- xfree (oldloc);
fprintf (stderr, "out of memory\n");
return false;
}
count = mbstowcs (progname_copy, progname, progsize + 1);
if (count == (size_t) -1)
{
- xfree (oldloc);
fprintf (stderr, "Could not convert python path to string\n");
return false;
}
- setlocale (LC_ALL, oldloc);
- xfree (oldloc);
+ setlocale (LC_ALL, oldloc.c_str ());
/* Note that Py_SetProgramName expects the string it is passed to
remain alive for the duration of the program's execution, so
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 20/23] Avoid some manual memory management in Python
2017-05-03 22:46 ` [RFA 20/23] Avoid some manual memory management in Python Tom Tromey
@ 2017-06-05 13:55 ` Pedro Alves
0 siblings, 0 replies; 59+ messages in thread
From: Pedro Alves @ 2017-06-05 13:55 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 05/03/2017 11:46 PM, Tom Tromey wrote:
> This changes a few places in the Python code to avoid manual memory
> management, in favor of letting std::string do the work.
Nice.
> -static char *
> +static std::string
> compute_python_string (struct command_line *l)
> {
This hunk in particular really simplifies things.
> - script = (char *) xmalloc (size + 1);
> - here = 0;
> - for (iter = l; iter; iter = iter->next)
> {
> - int len = strlen (iter->line);
> -
> - strcpy (&script[here], iter->line);
> - here += len;
> - script[here++] = '\n';
> + script += iter->line;
> + script += "\n";
Say:
script += '\n';
(i.e., call the character overload when append a
character, not the const char * overload.)
Otherwise OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 59+ messages in thread
* [RFA 12/23] More uses of scoped_restore
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (6 preceding siblings ...)
2017-05-03 22:46 ` [RFA 20/23] Avoid some manual memory management in Python Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-02 18:31 ` Pedro Alves
2017-05-03 22:46 ` [RFA 13/23] Replace tui_restore_gdbout with scoped_restore Tom Tromey
` (15 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
There were a few more places in gdb that could easily use
scoped_restore, replacing some cleanups.
2017-05-02 Tom Tromey <tom@tromey.com>
* reverse.c (exec_direction_default): Remove.
(exec_reverse_once): Use scoped_restore.
* remote.c (restore_remote_timeout): Remove.
(remote_flash_erase, remote_flash_write, remote_flash_done)
(readchar, remote_serial_write): Use scoped_restore.
* cli/cli-script.c (struct source_cleanup_lines_args)
(source_cleanup_lines): Remove.
(script_from_file): Use scoped_restore.
* cli/cli-cmds.c (source_verbose_cleanup): Remove.
(source_command): Use scoped_restore.
---
gdb/ChangeLog | 13 +++++++++++
gdb/cli/cli-cmds.c | 16 +-------------
gdb/cli/cli-script.c | 60 +++++++++++++++-----------------------------------
gdb/remote.c | 62 +++++++++++++++++-----------------------------------
gdb/reverse.c | 18 ++++-----------
5 files changed, 56 insertions(+), 113 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ddf87a3..d288de4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,18 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * reverse.c (exec_direction_default): Remove.
+ (exec_reverse_once): Use scoped_restore.
+ * remote.c (restore_remote_timeout): Remove.
+ (remote_flash_erase, remote_flash_write, remote_flash_done)
+ (readchar, remote_serial_write): Use scoped_restore.
+ * cli/cli-script.c (struct source_cleanup_lines_args)
+ (source_cleanup_lines): Remove.
+ (script_from_file): Use scoped_restore.
+ * cli/cli-cmds.c (source_verbose_cleanup): Remove.
+ (source_command): Use scoped_restore.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* utils.h (make_cleanup_free_so): Remove.
* utils.c (do_free_so, make_cleanup_free_so): Remove.
* solist.h (struct so_deleter): New.
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index c75dd16..3dc8b9c 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -633,26 +633,14 @@ source_script (const char *file, int from_tty)
source_script_with_search (file, from_tty, 0);
}
-/* Return the source_verbose global variable to its previous state
- on exit from the source command, by whatever means. */
-static void
-source_verbose_cleanup (void *old_value)
-{
- source_verbose = *(int *)old_value;
- xfree (old_value);
-}
-
static void
source_command (char *args, int from_tty)
{
- struct cleanup *old_cleanups;
char *file = args;
int *old_source_verbose = XNEW (int);
int search_path = 0;
- *old_source_verbose = source_verbose;
- old_cleanups = make_cleanup (source_verbose_cleanup,
- old_source_verbose);
+ scoped_restore save_source_verbose = make_scoped_restore (&source_verbose);
/* -v causes the source command to run in verbose mode.
-s causes the file to be searched in the source search path,
@@ -693,8 +681,6 @@ source_command (char *args, int from_tty)
}
source_script_with_search (file, from_tty, search_path);
-
- do_cleanups (old_cleanups);
}
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index e0e27ef..0aa1848 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -1584,58 +1584,34 @@ document_command (char *comname, int from_tty)
}
}
\f
-struct source_cleanup_lines_args
-{
- int old_line;
- const char *old_file;
-};
-
-static void
-source_cleanup_lines (void *args)
-{
- struct source_cleanup_lines_args *p =
- (struct source_cleanup_lines_args *) args;
-
- source_line_number = p->old_line;
- source_file_name = p->old_file;
-}
-
/* Used to implement source_command. */
void
script_from_file (FILE *stream, const char *file)
{
- struct cleanup *old_cleanups;
- struct source_cleanup_lines_args old_lines;
-
if (stream == NULL)
internal_error (__FILE__, __LINE__, _("called with NULL file pointer!"));
- old_lines.old_line = source_line_number;
- old_lines.old_file = source_file_name;
- old_cleanups = make_cleanup (source_cleanup_lines, &old_lines);
- source_line_number = 0;
- source_file_name = file;
-
- {
- scoped_restore save_async = make_scoped_restore (¤t_ui->async, 0);
+ scoped_restore restore_line_number
+ = make_scoped_restore (&source_line_number, 0);
+ scoped_restore resotre_file
+ = make_scoped_restore (&source_file_name, file);
- TRY
- {
- read_command_file (stream);
- }
- CATCH (e, RETURN_MASK_ERROR)
- {
- /* Re-throw the error, but with the file name information
- prepended. */
- throw_error (e.error,
- _("%s:%d: Error in sourced command file:\n%s"),
- source_file_name, source_line_number, e.message);
- }
- END_CATCH
- }
+ scoped_restore save_async = make_scoped_restore (¤t_ui->async, 0);
- do_cleanups (old_cleanups);
+ TRY
+ {
+ read_command_file (stream);
+ }
+ CATCH (e, RETURN_MASK_ERROR)
+ {
+ /* Re-throw the error, but with the file name information
+ prepended. */
+ throw_error (e.error,
+ _("%s:%d: Error in sourced command file:\n%s"),
+ source_file_name, source_line_number, e.message);
+ }
+ END_CATCH
}
/* Print the definition of user command C to STREAM. Or, if C is a
diff --git a/gdb/remote.c b/gdb/remote.c
index 1db2e57..ecd0ce0 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -72,6 +72,7 @@
#include "btrace.h"
#include "record-btrace.h"
#include <algorithm>
+#include "common/scoped_restore.h"
/* Temp hacks for tracepoint encoding migration. */
static char *target_buf;
@@ -8474,14 +8475,6 @@ remote_send_printf (const char *format, ...)
return packet_check_result (rs->buf);
}
-static void
-restore_remote_timeout (void *p)
-{
- int value = *(int *)p;
-
- remote_timeout = value;
-}
-
/* Flash writing can take quite some time. We'll set
effectively infinite timeout for flash operations.
In future, we'll need to decide on a better approach. */
@@ -8492,12 +8485,9 @@ remote_flash_erase (struct target_ops *ops,
ULONGEST address, LONGEST length)
{
int addr_size = gdbarch_addr_bit (target_gdbarch ()) / 8;
- int saved_remote_timeout = remote_timeout;
enum packet_result ret;
- struct cleanup *back_to = make_cleanup (restore_remote_timeout,
- &saved_remote_timeout);
-
- remote_timeout = remote_flash_timeout;
+ scoped_restore restore_timeout
+ = make_scoped_restore (&remote_timeout, remote_flash_timeout);
ret = remote_send_printf ("vFlashErase:%s,%s",
phex (address, addr_size),
@@ -8511,8 +8501,6 @@ remote_flash_erase (struct target_ops *ops,
default:
break;
}
-
- do_cleanups (back_to);
}
static enum target_xfer_status
@@ -8520,30 +8508,21 @@ remote_flash_write (struct target_ops *ops, ULONGEST address,
ULONGEST length, ULONGEST *xfered_len,
const gdb_byte *data)
{
- int saved_remote_timeout = remote_timeout;
- enum target_xfer_status ret;
- struct cleanup *back_to = make_cleanup (restore_remote_timeout,
- &saved_remote_timeout);
-
- remote_timeout = remote_flash_timeout;
- ret = remote_write_bytes_aux ("vFlashWrite:", address, data, length, 1,
- xfered_len,'X', 0);
- do_cleanups (back_to);
-
- return ret;
+ scoped_restore restore_timeout
+ = make_scoped_restore (&remote_timeout, remote_flash_timeout);
+ return remote_write_bytes_aux ("vFlashWrite:", address, data, length, 1,
+ xfered_len,'X', 0);
}
static void
remote_flash_done (struct target_ops *ops)
{
- int saved_remote_timeout = remote_timeout;
int ret;
- struct cleanup *back_to = make_cleanup (restore_remote_timeout,
- &saved_remote_timeout);
- remote_timeout = remote_flash_timeout;
+ scoped_restore restore_timeout
+ = make_scoped_restore (&remote_timeout, remote_flash_timeout);
+
ret = remote_send_printf ("vFlashDone");
- do_cleanups (back_to);
switch (ret)
{
@@ -8591,18 +8570,18 @@ readchar (int timeout)
{
int ch;
struct remote_state *rs = get_remote_state ();
- struct cleanup *old_chain;
- old_chain = make_cleanup_override_quit_handler (remote_serial_quit_handler);
-
- rs->got_ctrlc_during_io = 0;
+ {
+ scoped_restore restore_quit
+ = make_scoped_restore (&quit_handler, remote_serial_quit_handler);
- ch = serial_readchar (rs->remote_desc, timeout);
+ rs->got_ctrlc_during_io = 0;
- if (rs->got_ctrlc_during_io)
- set_quit_flag ();
+ ch = serial_readchar (rs->remote_desc, timeout);
- do_cleanups (old_chain);
+ if (rs->got_ctrlc_during_io)
+ set_quit_flag ();
+ }
if (ch >= 0)
return ch;
@@ -8635,7 +8614,8 @@ remote_serial_write (const char *str, int len)
struct remote_state *rs = get_remote_state ();
struct cleanup *old_chain;
- old_chain = make_cleanup_override_quit_handler (remote_serial_quit_handler);
+ scoped_restore restore_quit
+ = make_scoped_restore (&quit_handler, remote_serial_quit_handler);
rs->got_ctrlc_during_io = 0;
@@ -8647,8 +8627,6 @@ remote_serial_write (const char *str, int len)
if (rs->got_ctrlc_during_io)
set_quit_flag ();
-
- do_cleanups (old_chain);
}
/* Send the command in *BUF to the remote machine, and read the reply
diff --git a/gdb/reverse.c b/gdb/reverse.c
index 4080616..374a215 100644
--- a/gdb/reverse.c
+++ b/gdb/reverse.c
@@ -30,13 +30,6 @@
/* User interface:
reverse-step, reverse-next etc. */
-static void
-exec_direction_default (void *notused)
-{
- /* Return execution direction to default state. */
- execution_direction = EXEC_FORWARD;
-}
-
/* exec_reverse_once -- accepts an arbitrary gdb command (string),
and executes it with exec-direction set to 'reverse'.
@@ -45,9 +38,7 @@ exec_direction_default (void *notused)
static void
exec_reverse_once (const char *cmd, char *args, int from_tty)
{
- char *reverse_command;
enum exec_direction_kind dir = execution_direction;
- struct cleanup *old_chain;
if (dir == EXEC_REVERSE)
error (_("Already in reverse mode. Use '%s' or 'set exec-dir forward'."),
@@ -56,12 +47,11 @@ exec_reverse_once (const char *cmd, char *args, int from_tty)
if (!target_can_execute_reverse)
error (_("Target %s does not support this command."), target_shortname);
- reverse_command = xstrprintf ("%s %s", cmd, args ? args : "");
- old_chain = make_cleanup (exec_direction_default, NULL);
- make_cleanup (xfree, reverse_command);
+ std::string reverse_command = string_printf ("%s %s", cmd, args ? args : "");
+ scoped_restore restore_exec_dir
+ = make_scoped_restore (&execution_direction, EXEC_REVERSE);
execution_direction = EXEC_REVERSE;
- execute_command (reverse_command, from_tty);
- do_cleanups (old_chain);
+ execute_command (&reverse_command[0], from_tty);
}
static void
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 12/23] More uses of scoped_restore
2017-05-03 22:46 ` [RFA 12/23] More uses of scoped_restore Tom Tromey
@ 2017-06-02 18:31 ` Pedro Alves
0 siblings, 0 replies; 59+ messages in thread
From: Pedro Alves @ 2017-06-02 18:31 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 05/03/2017 11:46 PM, Tom Tromey wrote:
> @@ -8635,7 +8614,8 @@ remote_serial_write (const char *str, int len)
> struct remote_state *rs = get_remote_state ();
> struct cleanup *old_chain;
This cleanup variable can be removed.
>
> - old_chain = make_cleanup_override_quit_handler (remote_serial_quit_handler);
> + scoped_restore restore_quit
> + = make_scoped_restore (&quit_handler, remote_serial_quit_handler);
>
> rs->got_ctrlc_during_io = 0;
>
> @@ -56,12 +47,11 @@ exec_reverse_once (const char *cmd, char *args, int from_tty)
> if (!target_can_execute_reverse)
> error (_("Target %s does not support this command."), target_shortname);
>
> - reverse_command = xstrprintf ("%s %s", cmd, args ? args : "");
> - old_chain = make_cleanup (exec_direction_default, NULL);
> - make_cleanup (xfree, reverse_command);
> + std::string reverse_command = string_printf ("%s %s", cmd, args ? args : "");
> + scoped_restore restore_exec_dir
> + = make_scoped_restore (&execution_direction, EXEC_REVERSE);
> execution_direction = EXEC_REVERSE;
This assignment is now unnecessary.
OK with those changes.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 59+ messages in thread
* [RFA 13/23] Replace tui_restore_gdbout with scoped_restore
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (7 preceding siblings ...)
2017-05-03 22:46 ` [RFA 12/23] More uses of scoped_restore Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-02 18:34 ` Pedro Alves
2017-05-03 22:46 ` [RFA 04/23] Use gdb_file_up in fbsd-nat.c Tom Tromey
` (14 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This patch replaces tui_restore_gdbout (a cleaup function) with a use
of scoped_restore. This one is broken out into its own patch because
it might slightly change the behavior of gdb: it saves and restores
pagination_enabled, whereas the tui_restore_gdbout unconditionally set
pagination_enabled to 1; and I think this warrants closer review.
2017-05-02 Tom Tromey <tom@tromey.com>
* tui/tui-regs.c (tui_restore_gdbout): Remove.
(tui_register_format): Use scoped_restore.
---
gdb/ChangeLog | 5 +++++
gdb/tui/tui-regs.c | 26 ++++++--------------------
2 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d288de4..daa7dae 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * tui/tui-regs.c (tui_restore_gdbout): Remove.
+ (tui_register_format): Use scoped_restore.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* reverse.c (exec_direction_default): Remove.
(exec_reverse_once): Use scoped_restore.
* remote.c (restore_remote_timeout): Remove.
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 3f9a007..ade9cd6 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -708,13 +708,6 @@ TUI command to control the register window."), tuicmd);
** STATIC LOCAL FUNCTIONS **
******************************************/
-static void
-tui_restore_gdbout (void *ui)
-{
- gdb_stdout = (struct ui_file*) ui;
- pagination_enabled = 1;
-}
-
/* Get the register from the frame and return a printable
representation of it. */
@@ -722,17 +715,14 @@ static char *
tui_register_format (struct frame_info *frame, int regnum)
{
struct gdbarch *gdbarch = get_frame_arch (frame);
- struct ui_file *old_stdout;
- struct cleanup *cleanups;
- char *p, *s;
- char *ret;
string_file stream;
- pagination_enabled = 0;
- old_stdout = gdb_stdout;
- gdb_stdout = &stream;
- cleanups = make_cleanup (tui_restore_gdbout, (void*) old_stdout);
+ scoped_restore save_pagination
+ = make_scoped_restore (&pagination_enabled, 0);
+ scoped_restore save_stdout
+ = make_scoped_restore (&gdb_stdout, &stream);
+
gdbarch_print_registers_info (gdbarch, &stream, frame, regnum, 1);
/* Remove the possible \n. */
@@ -741,11 +731,7 @@ tui_register_format (struct frame_info *frame, int regnum)
str.resize (str.size () - 1);
/* Expand tabs into spaces, since ncurses on MS-Windows doesn't. */
- ret = tui_expand_tabs (str.c_str (), 0);
-
- do_cleanups (cleanups);
-
- return ret;
+ return tui_expand_tabs (str.c_str (), 0);
}
/* Get the register value from the given frame and format it for the
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 13/23] Replace tui_restore_gdbout with scoped_restore
2017-05-03 22:46 ` [RFA 13/23] Replace tui_restore_gdbout with scoped_restore Tom Tromey
@ 2017-06-02 18:34 ` Pedro Alves
0 siblings, 0 replies; 59+ messages in thread
From: Pedro Alves @ 2017-06-02 18:34 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 05/03/2017 11:46 PM, Tom Tromey wrote:
> This patch replaces tui_restore_gdbout (a cleaup function) with a use
> of scoped_restore. This one is broken out into its own patch because
> it might slightly change the behavior of gdb: it saves and restores
> pagination_enabled, whereas the tui_restore_gdbout unconditionally set
> pagination_enabled to 1; and I think this warrants closer review.
>
> 2017-05-02 Tom Tromey <tom@tromey.com>
>
> * tui/tui-regs.c (tui_restore_gdbout): Remove.
> (tui_register_format): Use scoped_restore.
I think I ran into this in last year's console work (in an
earlier prototype.). It just looks bogus to me to
unconditionally enable pagination.
Patch is OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 59+ messages in thread
* [RFA 04/23] Use gdb_file_up in fbsd-nat.c
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (8 preceding siblings ...)
2017-05-03 22:46 ` [RFA 13/23] Replace tui_restore_gdbout with scoped_restore Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-05-03 23:52 ` John Baldwin
2017-05-03 22:46 ` [RFA 17/23] Use a scoped_restore for user_call_depth Tom Tromey
` (13 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This updates fbsd-nat.c to use gdb_file_up. This removes a use of a
cleanup, and helps remove make_cleanup_fclose in a later patch.
I have no way to test this patch.
2017-05-02 Tom Tromey <tom@tromey.com>
* fbsd-nat.c (fbsd_find_memory_regions): Update.
---
gdb/ChangeLog | 4 ++++
gdb/fbsd-nat.c | 6 ++----
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 76c53be..799b727 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * fbsd-nat.c (fbsd_find_memory_regions): Update.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* cli/cli-cmds.c (find_and_open_script): Return gdb_file_p.
Remove "streamp" argument. Change "full_path" to a
unique_xmalloc_ptr.
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index ef5ad1e..3a84abc 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -160,7 +160,6 @@ fbsd_find_memory_regions (struct target_ops *self,
{
pid_t pid = ptid_get_pid (inferior_ptid);
char *mapfilename;
- FILE *mapfile;
unsigned long start, end, size;
char protection[4];
int read, write, exec;
@@ -168,17 +167,16 @@ fbsd_find_memory_regions (struct target_ops *self,
mapfilename = xstrprintf ("/proc/%ld/map", (long) pid);
cleanup = make_cleanup (xfree, mapfilename);
- mapfile = fopen (mapfilename, "r");
+ gdb_file_up mapfile = fopen (mapfilename, "r");
if (mapfile == NULL)
error (_("Couldn't open %s."), mapfilename);
- make_cleanup_fclose (mapfile);
if (info_verbose)
fprintf_filtered (gdb_stdout,
"Reading memory regions from %s\n", mapfilename);
/* Now iterate until end-of-file. */
- while (fbsd_read_mapping (mapfile, &start, &end, &protection[0]))
+ while (fbsd_read_mapping (mapfile.get (), &start, &end, &protection[0]))
{
size = end - start;
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 04/23] Use gdb_file_up in fbsd-nat.c
2017-05-03 22:46 ` [RFA 04/23] Use gdb_file_up in fbsd-nat.c Tom Tromey
@ 2017-05-03 23:52 ` John Baldwin
0 siblings, 0 replies; 59+ messages in thread
From: John Baldwin @ 2017-05-03 23:52 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
On Wednesday, May 03, 2017 04:46:07 PM Tom Tromey wrote:
> This updates fbsd-nat.c to use gdb_file_up. This removes a use of a
> cleanup, and helps remove make_cleanup_fclose in a later patch.
>
> I have no way to test this patch.
This looks fine to me. On the other hand, it's also not used on most
modern versions of FreeBSD (so I don't have a good way to test it either).
The new interface using sysctl has been available since FreeBSD 7.1
(released in January of 2009).
--
John Baldwin
^ permalink raw reply [flat|nested] 59+ messages in thread
* [RFA 17/23] Use a scoped_restore for user_call_depth
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (9 preceding siblings ...)
2017-05-03 22:46 ` [RFA 04/23] Use gdb_file_up in fbsd-nat.c Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-05 13:32 ` Pedro Alves
2017-05-03 22:46 ` [RFA 22/23] Make gdb_buildargv return a unique pointer Tom Tromey
` (12 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes execute_user_command to use a scoped_restore for
user_call_depth. This avoids a cleanup.
2017-05-02 Tom Tromey <tom@tromey.com>
* cli/cli-script.c (do_restore_user_call_depth): Remove.
(execute_user_command): Use scoped_restore.
---
gdb/ChangeLog | 5 +++++
gdb/cli/cli-script.c | 18 ++++--------------
2 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 45c5db7..873b404 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * cli/cli-script.c (do_restore_user_call_depth): Remove.
+ (execute_user_command): Use scoped_restore.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* top.h (in_user_command): Remove.
* top.c (in_user_command): Remove.
* cli/cli-script.c (do_restore_user_call_depth)
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 994f5c4..7f90d27 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -372,16 +372,6 @@ execute_cmd_post_hook (struct cmd_list_element *c)
}
}
-/* Execute the command in CMD. */
-static void
-do_restore_user_call_depth (void * call_depth)
-{
- int *depth = (int *) call_depth;
-
- (*depth)--;
-}
-
-
void
execute_user_command (struct cmd_list_element *c, char *args)
{
@@ -398,15 +388,15 @@ execute_user_command (struct cmd_list_element *c, char *args)
return;
scoped_user_args_level push_user_args (args);
+ scoped_restore restore_call_depth
+ = make_scoped_restore (&user_call_depth, user_call_depth + 1);
- if (++user_call_depth > max_user_call_depth)
+ if (user_call_depth > max_user_call_depth)
error (_("Max user call depth exceeded -- command aborted."));
- old_chain = make_cleanup (do_restore_user_call_depth, &user_call_depth);
-
/* Set the instream to 0, indicating execution of a
user-defined function. */
- make_cleanup (do_restore_instream_cleanup, ui->instream);
+ old_chain = make_cleanup (do_restore_instream_cleanup, ui->instream);
ui->instream = NULL;
scoped_restore save_async = make_scoped_restore (¤t_ui->async, 0);
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 17/23] Use a scoped_restore for user_call_depth
2017-05-03 22:46 ` [RFA 17/23] Use a scoped_restore for user_call_depth Tom Tromey
@ 2017-06-05 13:32 ` Pedro Alves
2017-06-05 16:56 ` Tom Tromey
0 siblings, 1 reply; 59+ messages in thread
From: Pedro Alves @ 2017-06-05 13:32 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 05/03/2017 11:46 PM, Tom Tromey wrote:
> -
> void
> execute_user_command (struct cmd_list_element *c, char *args)
> {
> @@ -398,15 +388,15 @@ execute_user_command (struct cmd_list_element *c, char *args)
> return;
>
> scoped_user_args_level push_user_args (args);
> + scoped_restore restore_call_depth
> + = make_scoped_restore (&user_call_depth, user_call_depth + 1);
>
> - if (++user_call_depth > max_user_call_depth)
> + if (user_call_depth > max_user_call_depth)
> error (_("Max user call depth exceeded -- command aborted."));
>
How about we remove the user_call_depth variable and check
user_args_stack.size () instead ?
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 59+ messages in thread
* [RFA 22/23] Make gdb_buildargv return a unique pointer
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (10 preceding siblings ...)
2017-05-03 22:46 ` [RFA 17/23] Use a scoped_restore for user_call_depth Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-05 16:21 ` Pedro Alves
2017-05-03 22:46 ` [RFA 11/23] Remove make_cleanup_free_so Tom Tromey
` (11 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This introduces gdb_argv_up, a unique pointer wrapping an "argv"
pointer; that is, a pointer to a NULL-terminated array of char*, where
both the array and each non-NULL element in the array are xmalloc'd.
This patch then changes gdb_buildargv to return a gdb_argv_up and
fixes all the users. It also changes skip.c to use gdb_buildargv
(previously it was the only direct caller of buildargv, other than
gdb_buildargv itself).
One future change that might make sense would be to add begin and end
functions, so that an argv could be iterated over. I didn't look into
this too much; and perhaps replacing some uses with
std::vector<std::string> would be even better.
2017-05-02 Tom Tromey <tom@tromey.com>
* utils.h (struct gdb_argv_deleter): New.
(gdb_argv_up): New typedef.
(gdb_buildargv): Change return type.
* utils.c (gdb_buildargv): Return gdb_argv_up.
* tracepoint.c (delete_trace_variable_command): Update.
* tracefile.c (tsave_command): Update.
* top.c (new_ui_command): Update.
* symmisc.c (maintenance_print_symbols)
(maintenance_print_msymbols, maintenance_expand_symtabs): Update.
* symfile.c (symbol_file_command, generic_load)
(remove_symbol_file_command): Update.
* stack.c (backtrace_command): Update.
* source.c (add_path, show_substitute_path_command)
(unset_substitute_path_command, set_substitute_path_command):
Update.
* skip.c (skip_command): Update. Use gdb_buildargv.
* ser-mingw.c (pipe_windows_open): Update.
* remote.c (extended_remote_run, remote_put_command)
(remote_get_command, remote_delete_command): Update.
* remote-sim.c (gdbsim_load, gdbsim_create_inferior)
(gdbsim_open): Update.
* python/py-cmd.c (gdbpy_string_to_argv): Update.
* psymtab.c (maintenance_print_psymbols): Update.
* procfs.c (procfs_info_proc): Update.
* interps.c (interpreter_exec_cmd): Update.
* infrun.c (handle_command): Update.
* inferior.c (add_inferior_command, clone_inferior_command):
Update.
* guile/scm-string.c (gdbscm_string_to_argv): Update.
* exec.c (exec_file_command): Update.
* compile/compile.c (build_argc_argv): Update.
* cli/cli-cmds.c (alias_command): Update.
---
gdb/ChangeLog | 35 +++++++++++++++++++++++++++++++++++
gdb/cli/cli-cmds.c | 31 +++++++++++++------------------
gdb/compile/compile.c | 2 +-
gdb/exec.c | 14 ++++----------
gdb/guile/scm-string.c | 4 +---
gdb/inferior.c | 26 ++++++++------------------
gdb/infrun.c | 8 ++------
gdb/interps.c | 13 ++-----------
gdb/procfs.c | 5 +++--
gdb/psymtab.c | 5 ++---
gdb/python/py-cmd.c | 9 ++-------
gdb/remote-sim.c | 10 +++++-----
gdb/remote.c | 30 ++++--------------------------
gdb/ser-mingw.c | 2 +-
gdb/skip.c | 8 +-------
gdb/source.c | 28 ++++++----------------------
gdb/stack.c | 2 +-
gdb/symfile.c | 22 ++++++++--------------
gdb/symmisc.c | 17 +++++------------
gdb/top.c | 10 ++--------
gdb/tracefile.c | 6 +++---
gdb/tracepoint.c | 7 +------
gdb/utils.c | 4 ++--
gdb/utils.h | 19 ++++++++++++++++++-
24 files changed, 130 insertions(+), 187 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 286d414..4dc3bb4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,40 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * utils.h (struct gdb_argv_deleter): New.
+ (gdb_argv_up): New typedef.
+ (gdb_buildargv): Change return type.
+ * utils.c (gdb_buildargv): Return gdb_argv_up.
+ * tracepoint.c (delete_trace_variable_command): Update.
+ * tracefile.c (tsave_command): Update.
+ * top.c (new_ui_command): Update.
+ * symmisc.c (maintenance_print_symbols)
+ (maintenance_print_msymbols, maintenance_expand_symtabs): Update.
+ * symfile.c (symbol_file_command, generic_load)
+ (remove_symbol_file_command): Update.
+ * stack.c (backtrace_command): Update.
+ * source.c (add_path, show_substitute_path_command)
+ (unset_substitute_path_command, set_substitute_path_command):
+ Update.
+ * skip.c (skip_command): Update. Use gdb_buildargv.
+ * ser-mingw.c (pipe_windows_open): Update.
+ * remote.c (extended_remote_run, remote_put_command)
+ (remote_get_command, remote_delete_command): Update.
+ * remote-sim.c (gdbsim_load, gdbsim_create_inferior)
+ (gdbsim_open): Update.
+ * python/py-cmd.c (gdbpy_string_to_argv): Update.
+ * psymtab.c (maintenance_print_psymbols): Update.
+ * procfs.c (procfs_info_proc): Update.
+ * interps.c (interpreter_exec_cmd): Update.
+ * infrun.c (handle_command): Update.
+ * inferior.c (add_inferior_command, clone_inferior_command):
+ Update.
+ * guile/scm-string.c (gdbscm_string_to_argv): Update.
+ * exec.c (exec_file_command): Update.
+ * compile/compile.c (build_argc_argv): Update.
+ * cli/cli-cmds.c (alias_command): Update.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* python/python.c (gdbpy_decode_line): Use unique_xmalloc_ptr.
2017-05-02 Tom Tromey <tom@tromey.com>
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 3dc8b9c..5ba0922 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1403,31 +1403,27 @@ alias_command (char *args, int from_tty)
{
int i, alias_argc, command_argc;
int abbrev_flag = 0;
- char *args2, *equals;
+ char *equals;
const char *alias, *command;
- char **alias_argv, **command_argv;
- struct cleanup *cleanup;
if (args == NULL || strchr (args, '=') == NULL)
alias_usage_error ();
- args2 = xstrdup (args);
- cleanup = make_cleanup (xfree, args2);
- equals = strchr (args2, '=');
- *equals = '\0';
- alias_argv = gdb_buildargv (args2);
- make_cleanup_freeargv (alias_argv);
- command_argv = gdb_buildargv (equals + 1);
- make_cleanup_freeargv (command_argv);
+ equals = strchr (args, '=');
+ std::string args2 (args, equals - args);
+
+ gdb_argv_up built_alias_argv = gdb_buildargv (args2.c_str ());
+ gdb_argv_up command_argv = gdb_buildargv (equals + 1);
- for (i = 0; alias_argv[i] != NULL; )
+ char **alias_argv = built_alias_argv.get ();
+ while (alias_argv[0] != NULL)
{
- if (strcmp (alias_argv[i], "-a") == 0)
+ if (strcmp (alias_argv[0], "-a") == 0)
{
++alias_argv;
abbrev_flag = 1;
}
- else if (strcmp (alias_argv[i], "--") == 0)
+ else if (strcmp (alias_argv[0], "--") == 0)
{
++alias_argv;
break;
@@ -1452,12 +1448,13 @@ alias_command (char *args, int from_tty)
}
alias_argc = countargv (alias_argv);
- command_argc = countargv (command_argv);
+ command_argc = countargv (command_argv.get ());
/* COMMAND must exist.
Reconstruct the command to remove any extraneous spaces,
for better error messages. */
- std::string command_string (argv_to_string (command_argv, command_argc));
+ std::string command_string (argv_to_string (command_argv.get (),
+ command_argc));
command = command_string.c_str ();
if (! valid_command_p (command))
error (_("Invalid command to alias to: %s"), command);
@@ -1514,8 +1511,6 @@ alias_command (char *args, int from_tty)
command_argv[command_argc - 1],
class_alias, abbrev_flag, c_command->prefixlist);
}
-
- do_cleanups (cleanup);
}
\f
/* Print a list of files and line numbers which a user may choose from
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 5269aaf..4994219 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -292,7 +292,7 @@ get_expr_block_and_pc (CORE_ADDR *pc)
static void
build_argc_argv (const char *s, int *argcp, char ***argvp)
{
- *argvp = gdb_buildargv (s);
+ *argvp = gdb_buildargv (s).release ();
*argcp = countargv (*argvp);
}
diff --git a/gdb/exec.c b/gdb/exec.c
index 2c9a74b..8fba0a1 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -410,7 +410,6 @@ exec_file_attach (const char *filename, int from_tty)
static void
exec_file_command (char *args, int from_tty)
{
- char **argv;
char *filename;
if (from_tty && target_has_execution
@@ -420,13 +419,11 @@ exec_file_command (char *args, int from_tty)
if (args)
{
- struct cleanup *cleanups;
-
/* Scan through the args and pick up the first non option arg
as the filename. */
- argv = gdb_buildargv (args);
- cleanups = make_cleanup_freeargv (argv);
+ gdb_argv_up built_argv = gdb_buildargv (args);
+ char **argv = built_argv.get ();
for (; (*argv != NULL) && (**argv == '-'); argv++)
{;
@@ -434,11 +431,8 @@ exec_file_command (char *args, int from_tty)
if (*argv == NULL)
error (_("No executable file name was specified"));
- filename = tilde_expand (*argv);
- make_cleanup (xfree, filename);
- exec_file_attach (filename, from_tty);
-
- do_cleanups (cleanups);
+ gdb::unique_xmalloc_ptr<char> filename (tilde_expand (*argv));
+ exec_file_attach (filename.get (), from_tty);
}
else
exec_file_attach (NULL, from_tty);
diff --git a/gdb/guile/scm-string.c b/gdb/guile/scm-string.c
index d97f583..c6432c1 100644
--- a/gdb/guile/scm-string.c
+++ b/gdb/guile/scm-string.c
@@ -241,7 +241,6 @@ static SCM
gdbscm_string_to_argv (SCM string_scm)
{
char *string;
- char **c_argv;
int i;
SCM result = SCM_EOL;
@@ -254,11 +253,10 @@ gdbscm_string_to_argv (SCM string_scm)
return SCM_EOL;
}
- c_argv = gdb_buildargv (string);
+ gdb_argv_up c_argv = gdb_buildargv (string);
for (i = 0; c_argv[i] != NULL; ++i)
result = scm_cons (gdbscm_scm_from_c_string (c_argv[i]), result);
- freeargv (c_argv);
xfree (string);
return scm_reverse_x (result, SCM_EOL);
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 002296f..1b467884 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -820,20 +820,17 @@ static void
add_inferior_command (char *args, int from_tty)
{
int i, copies = 1;
- char *exec = NULL;
- char **argv;
+ gdb::unique_xmalloc_ptr<char> exec;
symfile_add_flags add_flags = 0;
- struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
if (from_tty)
add_flags |= SYMFILE_VERBOSE;
if (args)
{
- argv = gdb_buildargv (args);
- make_cleanup_freeargv (argv);
+ gdb_argv_up built_argv = gdb_buildargv (args);
- for (; *argv != NULL; argv++)
+ for (char **argv = built_argv.get (); *argv != NULL; argv++)
{
if (**argv == '-')
{
@@ -849,8 +846,7 @@ add_inferior_command (char *args, int from_tty)
++argv;
if (!*argv)
error (_("No argument to -exec"));
- exec = tilde_expand (*argv);
- make_cleanup (xfree, exec);
+ exec.reset (tilde_expand (*argv));
}
}
else
@@ -874,12 +870,10 @@ add_inferior_command (char *args, int from_tty)
set_current_inferior (inf);
switch_to_thread (null_ptid);
- exec_file_attach (exec, from_tty);
- symbol_file_add_main (exec, add_flags);
+ exec_file_attach (exec.get (), from_tty);
+ symbol_file_add_main (exec.get (), add_flags);
}
}
-
- do_cleanups (old_chain);
}
/* clone-inferior [-copies N] [ID] */
@@ -888,15 +882,13 @@ static void
clone_inferior_command (char *args, int from_tty)
{
int i, copies = 1;
- char **argv;
struct inferior *orginf = NULL;
- struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
if (args)
{
- argv = gdb_buildargv (args);
- make_cleanup_freeargv (argv);
+ gdb_argv_up built_argv = gdb_buildargv (args);
+ char **argv = built_argv.get ();
for (; *argv != NULL; argv++)
{
if (**argv == '-')
@@ -967,8 +959,6 @@ clone_inferior_command (char *args, int from_tty)
switch_to_thread (null_ptid);
clone_program_space (pspace, orginf->pspace);
}
-
- do_cleanups (old_chain);
}
/* Print notices when new inferiors are created and die. */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index fcafdc1..03aad38 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8506,14 +8506,12 @@ sig_print_info (enum gdb_signal oursig)
static void
handle_command (char *args, int from_tty)
{
- char **argv;
int digits, wordlen;
int sigfirst, signum, siglast;
enum gdb_signal oursig;
int allsigs;
int nsigs;
unsigned char *sigs;
- struct cleanup *old_chain;
if (args == NULL)
{
@@ -8528,8 +8526,8 @@ handle_command (char *args, int from_tty)
/* Break the command line up into args. */
- argv = gdb_buildargv (args);
- old_chain = make_cleanup_freeargv (argv);
+ gdb_argv_up built_argv = gdb_buildargv (args);
+ char **argv = built_argv.get ();
/* Walk through the args, looking for signal oursigs, signal names, and
actions. Signal numbers and signal names may be interspersed with
@@ -8680,8 +8678,6 @@ Are you sure you want to change it? "),
break;
}
-
- do_cleanups (old_chain);
}
/* Complete the "handle" command. */
diff --git a/gdb/interps.c b/gdb/interps.c
index af86390..0512b38 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -407,21 +407,14 @@ interpreter_exec_cmd (char *args, int from_tty)
{
struct ui_interp_info *ui_interp = get_current_interp_info ();
struct interp *old_interp, *interp_to_use;
- char **prules = NULL;
- char **trule = NULL;
unsigned int nrules;
unsigned int i;
- struct cleanup *cleanup;
if (args == NULL)
error_no_arg (_("interpreter-exec command"));
- prules = gdb_buildargv (args);
- cleanup = make_cleanup_freeargv (prules);
-
- nrules = 0;
- for (trule = prules; *trule != NULL; trule++)
- nrules++;
+ gdb_argv_up prules = gdb_buildargv (args);
+ nrules = countargv (prules.get ());
if (nrules < 2)
error (_("usage: interpreter-exec <interpreter> [ <command> ... ]"));
@@ -446,8 +439,6 @@ interpreter_exec_cmd (char *args, int from_tty)
}
interp_set (old_interp, 0);
-
- do_cleanups (cleanup);
}
/* See interps.h. */
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 2aaee07..fa4fa1a 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -5108,10 +5108,11 @@ procfs_info_proc (struct target_ops *ops, const char *args,
}
old_chain = make_cleanup (null_cleanup, 0);
+ gdb_argv_up built_argv;
if (args)
{
- argv = gdb_buildargv (args);
- make_cleanup_freeargv (argv);
+ built_argv.reset (gdb_buildargv (args));
+ argv = built_argv.get ();
}
while (argv != NULL && *argv != NULL)
{
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index bb482ee..a85e512 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -1905,7 +1905,6 @@ dump_psymtab_addrmap (struct objfile *objfile, struct partial_symtab *psymtab,
static void
maintenance_print_psymbols (char *args, int from_tty)
{
- char **argv;
struct ui_file *outfile = gdb_stdout;
struct cleanup *cleanups;
char *address_arg = NULL, *source_arg = NULL, *objfile_arg = NULL;
@@ -1917,8 +1916,8 @@ maintenance_print_psymbols (char *args, int from_tty)
dont_repeat ();
- argv = gdb_buildargv (args);
- cleanups = make_cleanup_freeargv (argv);
+ gdb_argv_up argv = gdb_buildargv (args);
+ cleanups = make_cleanup (null_cleanup, NULL);
for (i = 0; argv != NULL && argv[i] != NULL; ++i)
{
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index 6203211..7bd25b4 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -763,7 +763,7 @@ gdbpy_string_to_argv (PyObject *self, PyObject *args)
if (*input != '\0')
{
- char **c_argv = gdb_buildargv (input);
+ gdb_argv_up c_argv = gdb_buildargv (input);
int i;
for (i = 0; c_argv[i] != NULL; ++i)
@@ -772,13 +772,8 @@ gdbpy_string_to_argv (PyObject *self, PyObject *args)
if (argp == NULL
|| PyList_Append (py_argv.get (), argp.get ()) < 0)
- {
- freeargv (c_argv);
- return NULL;
- }
+ return NULL;
}
-
- freeargv (c_argv);
}
return py_argv.release ();
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index fd1bc66..e932e87 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -574,8 +574,7 @@ gdbsim_load (struct target_ops *self, const char *args, int fromtty)
if (args == NULL)
error_no_arg (_("program to load"));
- argv = gdb_buildargv (args);
- make_cleanup_freeargv (argv);
+ gdb_argv_up argv = gdb_buildargv (args);
prog = tilde_expand (argv[0]);
@@ -632,6 +631,7 @@ gdbsim_create_inferior (struct target_ops *target, const char *exec_file,
remove_breakpoints ();
init_wait_for_inferior ();
+ gdb_argv_up built_argv;
if (exec_file != NULL)
{
len = strlen (exec_file) + 1 + allargs.size () + 1 + /*slop */ 10;
@@ -640,8 +640,8 @@ gdbsim_create_inferior (struct target_ops *target, const char *exec_file,
strcat (arg_buf, exec_file);
strcat (arg_buf, " ");
strcat (arg_buf, args);
- argv = gdb_buildargv (arg_buf);
- make_cleanup_freeargv (argv);
+ built_argv.reset (gdb_buildargv (arg_buf));
+ argv = built_argv.get ();
}
else
argv = NULL;
@@ -732,7 +732,7 @@ gdbsim_open (const char *args, int from_tty)
strcat (arg_buf, " "); /* 1 */
strcat (arg_buf, args);
}
- sim_argv = gdb_buildargv (arg_buf);
+ sim_argv = gdb_buildargv (arg_buf).release ();
init_callbacks ();
gdbsim_desc = sim_open (SIM_OPEN_DEBUG, &gdb_callback, exec_bfd, sim_argv);
diff --git a/gdb/remote.c b/gdb/remote.c
index ecd0ce0..a699387 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -9525,12 +9525,9 @@ extended_remote_run (const std::string &args)
if (!args.empty ())
{
- struct cleanup *back_to;
int i;
- char **argv;
- argv = gdb_buildargv (args.c_str ());
- back_to = make_cleanup_freeargv (argv);
+ gdb_argv_up argv = gdb_buildargv (args.c_str ());
for (i = 0; argv[i] != NULL; i++)
{
if (strlen (argv[i]) * 2 + 1 + len >= get_remote_packet_size ())
@@ -9539,7 +9536,6 @@ extended_remote_run (const std::string &args)
len += 2 * bin2hex ((gdb_byte *) argv[i], rs->buf + len,
strlen (argv[i]));
}
- do_cleanups (back_to);
}
rs->buf[len++] = '\0';
@@ -12035,58 +12031,40 @@ remote_file_delete (const char *remote_file, int from_tty)
static void
remote_put_command (char *args, int from_tty)
{
- struct cleanup *back_to;
- char **argv;
-
if (args == NULL)
error_no_arg (_("file to put"));
- argv = gdb_buildargv (args);
- back_to = make_cleanup_freeargv (argv);
+ gdb_argv_up argv = gdb_buildargv (args);
if (argv[0] == NULL || argv[1] == NULL || argv[2] != NULL)
error (_("Invalid parameters to remote put"));
remote_file_put (argv[0], argv[1], from_tty);
-
- do_cleanups (back_to);
}
static void
remote_get_command (char *args, int from_tty)
{
- struct cleanup *back_to;
- char **argv;
-
if (args == NULL)
error_no_arg (_("file to get"));
- argv = gdb_buildargv (args);
- back_to = make_cleanup_freeargv (argv);
+ gdb_argv_up argv = gdb_buildargv (args);
if (argv[0] == NULL || argv[1] == NULL || argv[2] != NULL)
error (_("Invalid parameters to remote get"));
remote_file_get (argv[0], argv[1], from_tty);
-
- do_cleanups (back_to);
}
static void
remote_delete_command (char *args, int from_tty)
{
- struct cleanup *back_to;
- char **argv;
-
if (args == NULL)
error_no_arg (_("file to delete"));
- argv = gdb_buildargv (args);
- back_to = make_cleanup_freeargv (argv);
+ gdb_argv_up argv = gdb_buildargv (args);
if (argv[0] == NULL || argv[1] != NULL)
error (_("Invalid parameters to remote delete"));
remote_file_delete (argv[0], from_tty);
-
- do_cleanups (back_to);
}
static void
diff --git a/gdb/ser-mingw.c b/gdb/ser-mingw.c
index 3f12458..429f89f 100644
--- a/gdb/ser-mingw.c
+++ b/gdb/ser-mingw.c
@@ -869,7 +869,7 @@ pipe_windows_open (struct serial *scb, const char *name)
if (name == NULL)
error_no_arg (_("child command"));
- argv = gdb_buildargv (name);
+ argv = gdb_buildargv (name).release ();
back_to = make_cleanup_freeargv (argv);
if (! argv[0] || argv[0][0] == '\0')
diff --git a/gdb/skip.c b/gdb/skip.c
index 33ed8d8..b830c6f 100644
--- a/gdb/skip.c
+++ b/gdb/skip.c
@@ -231,8 +231,6 @@ skip_command (char *arg, int from_tty)
const char *gfile = NULL;
const char *function = NULL;
const char *rfunction = NULL;
- char **argv;
- struct cleanup *cleanups;
struct skiplist_entry *e;
int i;
@@ -242,8 +240,7 @@ skip_command (char *arg, int from_tty)
return;
}
- argv = buildargv (arg);
- cleanups = make_cleanup_freeargv (argv);
+ gdb_argv_up argv = gdb_buildargv (arg);
for (i = 0; argv[i] != NULL; ++i)
{
@@ -290,7 +287,6 @@ skip_command (char *arg, int from_tty)
FUNCTION-NAME may be `foo (int)', and therefore we pass the
complete original arg to skip_function command as if the user
typed "skip function arg". */
- do_cleanups (cleanups);
skip_function_command (arg, from_tty);
return;
}
@@ -350,8 +346,6 @@ skip_command (char *arg, int from_tty)
lower_file_text, file_to_print);
}
}
-
- do_cleanups (cleanups);
}
static void
diff --git a/gdb/source.c b/gdb/source.c
index 3d477ad..7eef86c 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -484,16 +484,14 @@ add_path (char *dirname, char **which_path, int parse_separators)
if (parse_separators)
{
- char **argv, **argvp;
+ char **argvp;
/* This will properly parse the space and tab separators
and any quotes that may exist. */
- argv = gdb_buildargv (dirname);
+ gdb_argv_up argv = gdb_buildargv (dirname);
- for (argvp = argv; *argvp; argvp++)
+ for (argvp = argv.get (); *argvp; argvp++)
dirnames_to_char_ptr_vec_append (&dir_vec, *argvp);
-
- freeargv (argv);
}
else
VEC_safe_push (char_ptr, dir_vec, xstrdup (dirname));
@@ -1882,12 +1880,9 @@ static void
show_substitute_path_command (char *args, int from_tty)
{
struct substitute_path_rule *rule = substitute_path_rules;
- char **argv;
char *from = NULL;
- struct cleanup *cleanup;
- argv = gdb_buildargv (args);
- cleanup = make_cleanup_freeargv (argv);
+ gdb_argv_up argv = gdb_buildargv (args);
/* We expect zero or one argument. */
@@ -1911,8 +1906,6 @@ show_substitute_path_command (char *args, int from_tty)
printf_filtered (" `%s' -> `%s'.\n", rule->from, rule->to);
rule = rule->next;
}
-
- do_cleanups (cleanup);
}
/* Implement the "unset substitute-path" command. */
@@ -1921,14 +1914,12 @@ static void
unset_substitute_path_command (char *args, int from_tty)
{
struct substitute_path_rule *rule = substitute_path_rules;
- char **argv = gdb_buildargv (args);
+ gdb_argv_up argv = gdb_buildargv (args);
char *from = NULL;
int rule_found = 0;
- struct cleanup *cleanup;
/* This function takes either 0 or 1 argument. */
- cleanup = make_cleanup_freeargv (argv);
if (argv != NULL && argv[0] != NULL && argv[1] != NULL)
error (_("Incorrect usage, too many arguments in command"));
@@ -1966,8 +1957,6 @@ unset_substitute_path_command (char *args, int from_tty)
error (_("No substitution rule defined for `%s'"), from);
forget_cached_source_info ();
-
- do_cleanups (cleanup);
}
/* Add a new source path substitution rule. */
@@ -1975,12 +1964,9 @@ unset_substitute_path_command (char *args, int from_tty)
static void
set_substitute_path_command (char *args, int from_tty)
{
- char **argv;
struct substitute_path_rule *rule;
- struct cleanup *cleanup;
- argv = gdb_buildargv (args);
- cleanup = make_cleanup_freeargv (argv);
+ gdb_argv_up argv = gdb_buildargv (args);
if (argv == NULL || argv[0] == NULL || argv [1] == NULL)
error (_("Incorrect usage, too few arguments in command"));
@@ -2007,8 +1993,6 @@ set_substitute_path_command (char *args, int from_tty)
add_substitute_path_rule (argv[0], argv[1]);
forget_cached_source_info ();
-
- do_cleanups (cleanup);
}
\f
diff --git a/gdb/stack.c b/gdb/stack.c
index 7f8a51c..34e8aa5 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1877,7 +1877,7 @@ backtrace_command (char *arg, int from_tty)
char **argv;
int i;
- argv = gdb_buildargv (arg);
+ argv = gdb_buildargv (arg).release ();
make_cleanup_freeargv (argv);
argc = 0;
for (i = 0; argv[i]; i++)
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 846aabe..9482ddc 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1628,7 +1628,7 @@ symbol_file_command (char *args, int from_tty)
}
else
{
- char **argv = gdb_buildargv (args);
+ char **argv;
objfile_flags flags = OBJF_USERLOADED;
symfile_add_flags add_flags = 0;
struct cleanup *cleanups;
@@ -1637,7 +1637,8 @@ symbol_file_command (char *args, int from_tty)
if (from_tty)
add_flags |= SYMFILE_VERBOSE;
- cleanups = make_cleanup_freeargv (argv);
+ gdb_argv_up built_argv = gdb_buildargv (args);
+ argv = built_argv.get ();
while (*argv != NULL)
{
if (strcmp (*argv, "-readnow") == 0)
@@ -1655,8 +1656,6 @@ symbol_file_command (char *args, int from_tty)
if (name == NULL)
error (_("no symbol file name was specified"));
-
- do_cleanups (cleanups);
}
}
@@ -2050,25 +2049,23 @@ void
generic_load (const char *args, int from_tty)
{
char *filename;
- struct cleanup *old_cleanups = make_cleanup (null_cleanup, 0);
+ struct cleanup *old_cleanups;
struct load_section_data cbdata;
struct load_progress_data total_progress;
struct ui_out *uiout = current_uiout;
CORE_ADDR entry;
- char **argv;
memset (&cbdata, 0, sizeof (cbdata));
memset (&total_progress, 0, sizeof (total_progress));
cbdata.progress_data = &total_progress;
- make_cleanup (clear_memory_write_data, &cbdata.requests);
+ old_cleanups = make_cleanup (clear_memory_write_data, &cbdata.requests);
if (args == NULL)
error_no_arg (_("file to load"));
- argv = gdb_buildargv (args);
- make_cleanup_freeargv (argv);
+ gdb_argv_up argv = gdb_buildargv (args);
filename = tilde_expand (argv[0]);
make_cleanup (xfree, filename);
@@ -2216,7 +2213,6 @@ add_symbol_file_command (char *args, int from_tty)
int i;
int expecting_sec_name = 0;
int expecting_sec_addr = 0;
- char **argv;
struct objfile *objf;
objfile_flags flags = OBJF_USERLOADED | OBJF_SHARED;
symfile_add_flags add_flags = 0;
@@ -2243,8 +2239,7 @@ add_symbol_file_command (char *args, int from_tty)
if (args == NULL)
error (_("add-symbol-file takes a file name and an address"));
- argv = gdb_buildargv (args);
- make_cleanup_freeargv (argv);
+ gdb_argv_up argv = gdb_buildargv (args);
for (arg = argv[0], argcnt = 0; arg != NULL; arg = argv[++argcnt])
{
@@ -2364,7 +2359,6 @@ add_symbol_file_command (char *args, int from_tty)
static void
remove_symbol_file_command (char *args, int from_tty)
{
- char **argv;
struct objfile *objf = NULL;
struct cleanup *my_cleanups;
struct program_space *pspace = current_program_space;
@@ -2376,7 +2370,7 @@ remove_symbol_file_command (char *args, int from_tty)
my_cleanups = make_cleanup (null_cleanup, NULL);
- argv = gdb_buildargv (args);
+ gdb_argv_up argv = gdb_buildargv (args);
if (strcmp (argv[0], "-a") == 0)
{
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 32a5331..e3a512e 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -407,7 +407,6 @@ dump_symtab (struct symtab *symtab, struct ui_file *outfile)
static void
maintenance_print_symbols (char *args, int from_tty)
{
- char **argv;
struct ui_file *outfile = gdb_stdout;
struct cleanup *cleanups;
char *address_arg = NULL, *source_arg = NULL, *objfile_arg = NULL;
@@ -415,8 +414,8 @@ maintenance_print_symbols (char *args, int from_tty)
dont_repeat ();
- argv = gdb_buildargv (args);
- cleanups = make_cleanup_freeargv (argv);
+ gdb_argv_up argv = gdb_buildargv (args);
+ cleanups = make_cleanup (null_cleanup, NULL);
for (i = 0; argv != NULL && argv[i] != NULL; ++i)
{
@@ -709,7 +708,6 @@ print_symbol (void *args)
static void
maintenance_print_msymbols (char *args, int from_tty)
{
- char **argv;
struct ui_file *outfile = gdb_stdout;
struct cleanup *cleanups;
char *objfile_arg = NULL;
@@ -718,8 +716,8 @@ maintenance_print_msymbols (char *args, int from_tty)
dont_repeat ();
- argv = gdb_buildargv (args);
- cleanups = make_cleanup_freeargv (argv);
+ gdb_argv_up argv = gdb_buildargv (args);
+ cleanups = make_cleanup (null_cleanup, NULL);
for (i = 0; argv != NULL && argv[i] != NULL; ++i)
{
@@ -944,14 +942,11 @@ maintenance_expand_symtabs (char *args, int from_tty)
{
struct program_space *pspace;
struct objfile *objfile;
- struct cleanup *cleanups;
- char **argv;
char *regexp = NULL;
/* We use buildargv here so that we handle spaces in the regexp
in a way that allows adding more arguments later. */
- argv = gdb_buildargv (args);
- cleanups = make_cleanup_freeargv (argv);
+ gdb_argv_up argv = gdb_buildargv (args);
if (argv != NULL)
{
@@ -988,8 +983,6 @@ maintenance_expand_symtabs (char *args, int from_tty)
ALL_DOMAIN);
}
}
-
- do_cleanups (cleanups);
}
\f
diff --git a/gdb/top.c b/gdb/top.c
index 6038366..a51b59f 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -364,17 +364,14 @@ new_ui_command (char *args, int from_tty)
int i;
int res;
int argc;
- char **argv;
const char *interpreter_name;
const char *tty_name;
- struct cleanup *success_chain;
struct cleanup *failure_chain;
dont_repeat ();
- argv = gdb_buildargv (args);
- success_chain = make_cleanup_freeargv (argv);
- argc = countargv (argv);
+ gdb_argv_up argv = gdb_buildargv (args);
+ argc = countargv (argv.get ());
if (argc < 2)
error (_("usage: new-ui <interpreter> <tty>"));
@@ -407,9 +404,6 @@ new_ui_command (char *args, int from_tty)
stream[2].release ();
discard_cleanups (failure_chain);
-
- /* This restores the previous UI and frees argv. */
- do_cleanups (success_chain);
}
printf_unfiltered ("New UI allocated\n");
diff --git a/gdb/tracefile.c b/gdb/tracefile.c
index cc90945..2dce0d9 100644
--- a/gdb/tracefile.c
+++ b/gdb/tracefile.c
@@ -318,8 +318,8 @@ tsave_command (char *args, int from_tty)
if (args == NULL)
error_no_arg (_("file in which to save trace data"));
- argv = gdb_buildargv (args);
- back_to = make_cleanup_freeargv (argv);
+ gdb_argv_up built_argv = gdb_buildargv (args);
+ argv = built_argv.get ();
for (; *argv; ++argv)
{
@@ -341,7 +341,7 @@ tsave_command (char *args, int from_tty)
else
writer = tfile_trace_file_writer_new ();
- make_cleanup (trace_file_writer_xfree, writer);
+ back_to = make_cleanup (trace_file_writer_xfree, writer);
trace_save (filename, writer, target_does_save);
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 3a39ebc..f569c86 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -448,8 +448,6 @@ static void
delete_trace_variable_command (char *args, int from_tty)
{
int ix;
- char **argv;
- struct cleanup *back_to;
if (args == NULL)
{
@@ -460,8 +458,7 @@ delete_trace_variable_command (char *args, int from_tty)
return;
}
- argv = gdb_buildargv (args);
- back_to = make_cleanup_freeargv (argv);
+ gdb_argv_up argv = gdb_buildargv (args);
for (ix = 0; argv[ix] != NULL; ix++)
{
@@ -471,8 +468,6 @@ delete_trace_variable_command (char *args, int from_tty)
warning (_("Name \"%s\" not prefixed with '$', ignoring"), argv[ix]);
}
- do_cleanups (back_to);
-
dont_repeat ();
}
diff --git a/gdb/utils.c b/gdb/utils.c
index 7d0b5f4..b631ed0 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2914,14 +2914,14 @@ ldirname (const char *filename)
Therefore, the returned value is guaranteed to be non-NULL,
unless the parameter itself is NULL. */
-char **
+gdb_argv_up
gdb_buildargv (const char *s)
{
char **argv = buildargv (s);
if (s != NULL && argv == NULL)
malloc_failure (0);
- return argv;
+ return gdb_argv_up (argv);
}
int
diff --git a/gdb/utils.h b/gdb/utils.h
index 8f545c6..dd7c75e7 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -60,7 +60,24 @@ extern int parse_pid_to_attach (const char *args);
extern int parse_escape (struct gdbarch *, const char **);
-char **gdb_buildargv (const char *);
+/* A deleter for an argv array. This is an array where each element,
+ up to the first NULL, is xmalloc'd, and the array itself is
+ xmalloc'd. */
+struct gdb_argv_deleter
+{
+ void operator() (char **argv) const
+ {
+ freeargv (argv);
+ }
+};
+
+/* A unique pointer to an argv array. */
+typedef std::unique_ptr<char *[], gdb_argv_deleter> gdb_argv_up;
+
+/* A wrapper for libiberty's buildargv that handles memory allocation
+ failures in the gdb style. */
+gdb_argv_up gdb_buildargv (const char *);
+
\f
/* Cleanup utilities. */
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 22/23] Make gdb_buildargv return a unique pointer
2017-05-03 22:46 ` [RFA 22/23] Make gdb_buildargv return a unique pointer Tom Tromey
@ 2017-06-05 16:21 ` Pedro Alves
2017-07-23 17:26 ` Tom Tromey
0 siblings, 1 reply; 59+ messages in thread
From: Pedro Alves @ 2017-06-05 16:21 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 05/03/2017 11:46 PM, Tom Tromey wrote:
> This introduces gdb_argv_up, a unique pointer wrapping an "argv"
> pointer; that is, a pointer to a NULL-terminated array of char*, where
> both the array and each non-NULL element in the array are xmalloc'd.
>
> This patch then changes gdb_buildargv to return a gdb_argv_up and
> fixes all the users. It also changes skip.c to use gdb_buildargv
> (previously it was the only direct caller of buildargv, other than
> gdb_buildargv itself).
>
> One future change that might make sense would be to add begin and end
> functions, so that an argv could be iterated over. I didn't look into
> this too much; and perhaps replacing some uses with
> std::vector<std::string> would be even better.
I suspect that making gdb_argv_up a class (class gdb_argv) that wraps
the array instead of making it a unique pointer may make users a
little bit clearer. I.e., gdb_buildargv would be converted to a
ctor, and we'd add e.g., a "count()" and "release()" methods.
Meanwhile ...
>
> 2017-05-02 Tom Tromey <tom@tromey.com>
>
> * utils.h (struct gdb_argv_deleter): New.
> (gdb_argv_up): New typedef.
> (gdb_buildargv): Change return type.
> * utils.c (gdb_buildargv): Return gdb_argv_up.
> * tracepoint.c (delete_trace_variable_command): Update.
> * tracefile.c (tsave_command): Update.
> * top.c (new_ui_command): Update.
> * symmisc.c (maintenance_print_symbols)
> (maintenance_print_msymbols, maintenance_expand_symtabs): Update.
> * symfile.c (symbol_file_command, generic_load)
> (remove_symbol_file_command): Update.
> * stack.c (backtrace_command): Update.
> * source.c (add_path, show_substitute_path_command)
> (unset_substitute_path_command, set_substitute_path_command):
> Update.
> * skip.c (skip_command): Update. Use gdb_buildargv.
> * ser-mingw.c (pipe_windows_open): Update.
> * remote.c (extended_remote_run, remote_put_command)
> (remote_get_command, remote_delete_command): Update.
> * remote-sim.c (gdbsim_load, gdbsim_create_inferior)
> (gdbsim_open): Update.
> * python/py-cmd.c (gdbpy_string_to_argv): Update.
> * psymtab.c (maintenance_print_psymbols): Update.
> * procfs.c (procfs_info_proc): Update.
> * interps.c (interpreter_exec_cmd): Update.
> * infrun.c (handle_command): Update.
> * inferior.c (add_inferior_command, clone_inferior_command):
> Update.
> * guile/scm-string.c (gdbscm_string_to_argv): Update.
> * exec.c (exec_file_command): Update.
> * compile/compile.c (build_argc_argv): Update.
> * cli/cli-cmds.c (alias_command): Update.
LGTM.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [RFA 22/23] Make gdb_buildargv return a unique pointer
2017-06-05 16:21 ` Pedro Alves
@ 2017-07-23 17:26 ` Tom Tromey
0 siblings, 0 replies; 59+ messages in thread
From: Tom Tromey @ 2017-07-23 17:26 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> I suspect that making gdb_argv_up a class (class gdb_argv) that wraps
Pedro> the array instead of making it a unique pointer may make users a
Pedro> little bit clearer. I.e., gdb_buildargv would be converted to a
Pedro> ctor, and we'd add e.g., a "count()" and "release()" methods.
I did this. It does clean things up a bit, particularly since we can
provide begin/end methods and then use foreach to iterate in some cases.
Tom
^ permalink raw reply [flat|nested] 59+ messages in thread
* [RFA 11/23] Remove make_cleanup_free_so
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (11 preceding siblings ...)
2017-05-03 22:46 ` [RFA 22/23] Make gdb_buildargv return a unique pointer Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-02 18:25 ` Pedro Alves
2017-05-03 22:46 ` [RFA 14/23] Use unique_xmalloc_ptr in jit.c Tom Tromey
` (10 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
make_cleanup_free_so is used in a single spot. This patch introduces
a unique pointer wrapper for struct so_list, and changes this spot to
use it.
2017-05-02 Tom Tromey <tom@tromey.com>
* utils.h (make_cleanup_free_so): Remove.
* utils.c (do_free_so, make_cleanup_free_so): Remove.
* solist.h (struct so_deleter): New.
(so_list_up): New typedef.
* solib-svr4.c (svr4_read_so_list): Use so_list_up.
---
gdb/ChangeLog | 8 ++++++++
gdb/solib-svr4.c | 24 ++++++------------------
gdb/solist.h | 12 ++++++++++++
gdb/utils.c | 18 ------------------
gdb/utils.h | 3 ---
5 files changed, 26 insertions(+), 39 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1c3b738..ddf87a3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * utils.h (make_cleanup_free_so): Remove.
+ * utils.c (do_free_so, make_cleanup_free_so): Remove.
+ * solist.h (struct so_deleter): New.
+ (so_list_up): New typedef.
+ * solib-svr4.c (svr4_read_so_list): Use so_list_up.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* utils.h (make_cleanup_restore_current_language): Remove.
* utils.c (do_restore_current_language)
(make_cleanup_restore_current_language): Remove.
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 080fd79..f99e2e2 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1350,21 +1350,15 @@ svr4_read_so_list (CORE_ADDR lm, CORE_ADDR prev_lm,
for (; lm != 0; prev_lm = lm, lm = next_lm)
{
- struct so_list *newobj;
- struct cleanup *old_chain;
int errcode;
char *buffer;
- newobj = XCNEW (struct so_list);
- old_chain = make_cleanup_free_so (newobj);
+ so_list_up newobj (XCNEW (struct so_list));
lm_info_svr4 *li = lm_info_read (lm);
newobj->lm_info = li;
if (li == NULL)
- {
- do_cleanups (old_chain);
- return 0;
- }
+ return 0;
next_lm = li->l_next;
@@ -1373,7 +1367,6 @@ svr4_read_so_list (CORE_ADDR lm, CORE_ADDR prev_lm,
warning (_("Corrupted shared library list: %s != %s"),
paddress (target_gdbarch (), prev_lm),
paddress (target_gdbarch (), li->l_prev));
- do_cleanups (old_chain);
return 0;
}
@@ -1388,7 +1381,6 @@ svr4_read_so_list (CORE_ADDR lm, CORE_ADDR prev_lm,
first_l_name = li->l_name;
info->main_lm_addr = li->lm_addr;
- do_cleanups (old_chain);
continue;
}
@@ -1404,7 +1396,6 @@ svr4_read_so_list (CORE_ADDR lm, CORE_ADDR prev_lm,
if (first_l_name == 0 || li->l_name != first_l_name)
warning (_("Can't read pathname for load map: %s."),
safe_strerror (errcode));
- do_cleanups (old_chain);
continue;
}
@@ -1416,15 +1407,12 @@ svr4_read_so_list (CORE_ADDR lm, CORE_ADDR prev_lm,
/* If this entry has no name, or its name matches the name
for the main executable, don't include it in the list. */
if (! newobj->so_name[0] || match_main (newobj->so_name))
- {
- do_cleanups (old_chain);
- continue;
- }
+ continue;
- discard_cleanups (old_chain);
newobj->next = 0;
- **link_ptr_ptr = newobj;
- *link_ptr_ptr = &newobj->next;
+ /* Don't free it now. */
+ **link_ptr_ptr = newobj.release ();
+ *link_ptr_ptr = &(**link_ptr_ptr)->next;
}
return 1;
diff --git a/gdb/solist.h b/gdb/solist.h
index 54c9902..5eb2d39 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -179,6 +179,18 @@ struct target_so_ops
/* Free the memory associated with a (so_list *). */
void free_so (struct so_list *so);
+/* A deleter that calls free_so. */
+struct so_deleter
+{
+ void operator() (struct so_list *so) const
+ {
+ free_so (so);
+ }
+};
+
+/* A unique pointer to a so_list. */
+typedef std::unique_ptr<so_list, so_deleter> so_list_up;
+
/* Return address of first so_list entry in master shared object list. */
struct so_list *master_so_list (void);
diff --git a/gdb/utils.c b/gdb/utils.c
index 1a66896..7d0b5f4 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -289,24 +289,6 @@ make_cleanup_value_free (struct value *value)
return make_cleanup (do_value_free, value);
}
-/* Helper for make_cleanup_free_so. */
-
-static void
-do_free_so (void *arg)
-{
- struct so_list *so = (struct so_list *) arg;
-
- free_so (so);
-}
-
-/* Make cleanup handler calling free_so for SO. */
-
-struct cleanup *
-make_cleanup_free_so (struct so_list *so)
-{
- return make_cleanup (do_free_so, so);
-}
-
/* Helper function for make_cleanup_clear_parser_state. */
static void
diff --git a/gdb/utils.h b/gdb/utils.h
index 1f01bdd..8f545c6 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -88,9 +88,6 @@ extern struct cleanup *make_cleanup_unpush_target (struct target_ops *ops);
extern struct cleanup *make_cleanup_value_free_to_mark (struct value *);
extern struct cleanup *make_cleanup_value_free (struct value *);
-struct so_list;
-extern struct cleanup *make_cleanup_free_so (struct so_list *so);
-
/* A deleter for a hash table. */
struct htab_deleter
{
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* [RFA 14/23] Use unique_xmalloc_ptr in jit.c
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (12 preceding siblings ...)
2017-05-03 22:46 ` [RFA 11/23] Remove make_cleanup_free_so Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-02 18:42 ` Pedro Alves
2017-05-03 22:46 ` [RFA 08/23] Remove an unlink cleanup Tom Tromey
` (9 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This removes some cleanups from jit.c by using unique_xmalloc_ptr
instead.
2017-05-02 Tom Tromey <tom@tromey.com>
* jit.c (jit_reader_load_command): Use unique_xmalloc_ptr.
---
gdb/ChangeLog | 4 ++++
gdb/jit.c | 20 +++++++-------------
2 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index daa7dae..03e733b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * jit.c (jit_reader_load_command): Use unique_xmalloc_ptr.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* tui/tui-regs.c (tui_restore_gdbout): Remove.
(tui_register_format): Use scoped_restore.
diff --git a/gdb/jit.c b/gdb/jit.c
index ddf1005..00975df 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -211,29 +211,23 @@ jit_reader_load (const char *file_name)
static void
jit_reader_load_command (char *args, int from_tty)
{
- char *so_name;
- struct cleanup *prev_cleanup;
-
if (args == NULL)
error (_("No reader name provided."));
- args = tilde_expand (args);
- prev_cleanup = make_cleanup (xfree, args);
+ gdb::unique_xmalloc_ptr<char> file (tilde_expand (args));
if (loaded_jit_reader != NULL)
error (_("JIT reader already loaded. Run jit-reader-unload first."));
- if (IS_ABSOLUTE_PATH (args))
- so_name = args;
+ gdb::unique_xmalloc_ptr<char> so_name;
+ if (IS_ABSOLUTE_PATH (file.get ()))
+ so_name = std::move (file);
else
- {
- so_name = xstrprintf ("%s%s%s", jit_reader_dir, SLASH_STRING, args);
- make_cleanup (xfree, so_name);
- }
+ so_name.reset (xstrprintf ("%s%s%s", jit_reader_dir, SLASH_STRING,
+ file.get ()));
- loaded_jit_reader = jit_reader_load (so_name);
+ loaded_jit_reader = jit_reader_load (so_name.get ());
reinit_frame_cache ();
jit_inferior_created_hook ();
- do_cleanups (prev_cleanup);
}
/* Provides the jit-reader-unload command. */
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 14/23] Use unique_xmalloc_ptr in jit.c
2017-05-03 22:46 ` [RFA 14/23] Use unique_xmalloc_ptr in jit.c Tom Tromey
@ 2017-06-02 18:42 ` Pedro Alves
2017-06-05 13:09 ` Tom Tromey
0 siblings, 1 reply; 59+ messages in thread
From: Pedro Alves @ 2017-06-02 18:42 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 05/03/2017 11:46 PM, Tom Tromey wrote:
> static void
> jit_reader_load_command (char *args, int from_tty)
> {
> - char *so_name;
> - struct cleanup *prev_cleanup;
> -
> if (args == NULL)
> error (_("No reader name provided."));
> - args = tilde_expand (args);
> - prev_cleanup = make_cleanup (xfree, args);
> + gdb::unique_xmalloc_ptr<char> file (tilde_expand (args));
>
> if (loaded_jit_reader != NULL)
> error (_("JIT reader already loaded. Run jit-reader-unload first."));
>
> - if (IS_ABSOLUTE_PATH (args))
> - so_name = args;
> + gdb::unique_xmalloc_ptr<char> so_name;
> + if (IS_ABSOLUTE_PATH (file.get ()))
> + so_name = std::move (file);
I think we don't really need two unique pointers, and then
moving. I.e., this should do, I think:
if (loaded_jit_reader != NULL)
error (_("JIT reader already loaded. Run jit-reader-unload first."));
gdb::unique_xmalloc_ptr<char> so_name (tilde_expand (args));
if (!IS_ABSOLUTE_PATH (so_name.get ()))
so_name.reset (xstrprintf ("%s%s%s", jit_reader_dir, SLASH_STRING,
so_name.get ()));
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 59+ messages in thread
* [RFA 08/23] Remove an unlink cleanup
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (13 preceding siblings ...)
2017-05-03 22:46 ` [RFA 14/23] Use unique_xmalloc_ptr in jit.c Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-02 17:37 ` Pedro Alves
2017-05-03 22:46 ` [RFA 21/23] Remove a cleanup in Python Tom Tromey
` (8 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
compile/compile.c had its own cleanup to unlink a file. This patch
replaces this cleanup with gdb::unlinker.
2017-05-02 Tom Tromey <tom@tromey.com>
* compile/compile.c (cleanup_unlink_file): Remove.
(compile_to_object): Use gdb::unlinker.
(eval_compile_command): Likewise.
---
gdb/ChangeLog | 6 ++++++
gdb/compile/compile.c | 38 +++++++++++++++++++-------------------
2 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2f1219f..2d0d71d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * compile/compile.c (cleanup_unlink_file): Remove.
+ (compile_to_object): Use gdb::unlinker.
+ (eval_compile_command): Likewise.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* utils.h (make_cleanup_fclose): Remove.
* utils.c (do_fclose_cleanup, make_cleanup_fclose): Remove.
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index d8c505f..5269aaf 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -39,6 +39,8 @@
#include "osabi.h"
#include "gdb_wait.h"
#include "valprint.h"
+#include "common/gdb_optional.h"
+#include "common/gdb_unlinker.h"
\f
@@ -427,16 +429,6 @@ cleanup_compile_instance (void *arg)
inst->destroy (inst);
}
-/* A cleanup function to unlink a file. */
-
-static void
-cleanup_unlink_file (void *arg)
-{
- const char *filename = (const char *) arg;
-
- unlink (filename);
-}
-
/* A helper function suitable for use as the "print_callback" in the
compiler object. */
@@ -455,7 +447,7 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
enum compile_i_scope_types scope)
{
struct compile_instance *compiler;
- struct cleanup *cleanup, *inner_cleanup;
+ struct cleanup *cleanup;
const struct block *expr_block;
CORE_ADDR trash_pc, expr_pc;
int argc;
@@ -547,12 +539,15 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
compile_file_names fnames = get_new_file_names ();
+ gdb::optional<gdb::unlinker> source_remover;
+
{
gdb_file_up src = gdb_fopen_cloexec (fnames.source_file (), "w");
if (src == NULL)
perror_with_name (_("Could not open source file for writing"));
- inner_cleanup = make_cleanup (cleanup_unlink_file,
- (void *) fnames.source_file ());
+
+ source_remover.emplace (fnames.source_file ());
+
if (fputs (code.c_str (), src.get ()) == EOF)
perror_with_name (_("Could not write to source file"));
}
@@ -572,7 +567,9 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
fprintf_unfiltered (gdb_stdlog, "object file produced: %s\n\n",
fnames.object_file ());
- discard_cleanups (inner_cleanup);
+ /* Keep the source file. */
+ source_remover->keep ();
+
do_cleanups (cleanup);
return fnames;
@@ -594,14 +591,13 @@ void
eval_compile_command (struct command_line *cmd, const char *cmd_string,
enum compile_i_scope_types scope, void *scope_data)
{
- struct cleanup *cleanup_unlink;
struct compile_module *compile_module;
compile_file_names fnames = compile_to_object (cmd, cmd_string, scope);
- cleanup_unlink = make_cleanup (cleanup_unlink_file,
- (void *) fnames.object_file ());
- make_cleanup (cleanup_unlink_file, (void *) fnames.source_file ());
+ gdb::unlinker object_remover (fnames.object_file ());
+ gdb::unlinker source_remover (fnames.source_file ());
+
compile_module = compile_object_load (fnames, scope, scope_data);
if (compile_module == NULL)
{
@@ -610,7 +606,11 @@ eval_compile_command (struct command_line *cmd, const char *cmd_string,
COMPILE_I_PRINT_VALUE_SCOPE, scope_data);
return;
}
- discard_cleanups (cleanup_unlink);
+
+ /* Keep the files. */
+ source_remover.keep ();
+ object_remover.keep ();
+
compile_object_run (compile_module);
}
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* [RFA 21/23] Remove a cleanup in Python
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (14 preceding siblings ...)
2017-05-03 22:46 ` [RFA 08/23] Remove an unlink cleanup Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-05 13:56 ` Pedro Alves
2017-05-03 22:46 ` [RFA 16/23] Remove in_user_command Tom Tromey
` (7 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This removes cleanups from gdbpy_decode_line, in favor of a use of
unique_xmalloc_ptr.
2017-05-02 Tom Tromey <tom@tromey.com>
* python/python.c (gdbpy_decode_line): Use unique_xmalloc_ptr.
---
gdb/ChangeLog | 4 ++++
gdb/python/python.c | 30 +++++++-----------------------
2 files changed, 11 insertions(+), 23 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fc88456..286d414 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * python/python.c (gdbpy_decode_line): Use unique_xmalloc_ptr.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* python/python.c (compute_python_string): Return std::string.
(gdbpy_eval_from_control_command): Update.
(do_start_initialization): Use std::string.
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 17ab04e..8e8cdbf 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -652,7 +652,6 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
appease gcc. */
struct symtab_and_line sal;
char *arg = NULL;
- struct cleanup *cleanups;
gdbpy_ref<> result;
gdbpy_ref<> unparsed;
event_location_up location;
@@ -660,8 +659,6 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
if (! PyArg_ParseTuple (args, "|s", &arg))
return NULL;
- cleanups = make_cleanup (null_cleanup, NULL);
-
sals.sals = NULL;
if (arg != NULL)
@@ -685,12 +682,13 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
}
END_CATCH
+ /* Ensure that the sals data is freed, when needed. */
+ gdb::unique_xmalloc_ptr<struct symtab_and_line> free_sals;
if (sals.sals != NULL && sals.sals != &sal)
- make_cleanup (xfree, sals.sals);
+ free_sals.reset (sals.sals);
if (except.reason < 0)
{
- do_cleanups (cleanups);
/* We know this will always throw. */
gdbpy_convert_exception (except);
return NULL;
@@ -702,20 +700,14 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
result.reset (PyTuple_New (sals.nelts));
if (result == NULL)
- {
- do_cleanups (cleanups);
- return NULL;
- }
+ return NULL;
for (i = 0; i < sals.nelts; ++i)
{
PyObject *obj;
obj = symtab_and_line_to_sal_object (sals.sals[i]);
if (! obj)
- {
- do_cleanups (cleanups);
- return NULL;
- }
+ return NULL;
PyTuple_SetItem (result.get (), i, obj);
}
@@ -728,19 +720,13 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
gdbpy_ref<> return_result (PyTuple_New (2));
if (return_result == NULL)
- {
- do_cleanups (cleanups);
- return NULL;
- }
+ return NULL;
if (arg != NULL && strlen (arg) > 0)
{
unparsed.reset (PyString_FromString (arg));
if (unparsed == NULL)
- {
- do_cleanups (cleanups);
- return NULL;
- }
+ return NULL;
}
else
{
@@ -751,8 +737,6 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
PyTuple_SetItem (return_result.get (), 0, unparsed.release ());
PyTuple_SetItem (return_result.get (), 1, result.release ());
- do_cleanups (cleanups);
-
return return_result.release ();
}
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* [RFA 16/23] Remove in_user_command
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (15 preceding siblings ...)
2017-05-03 22:46 ` [RFA 21/23] Remove a cleanup in Python Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-05 13:27 ` Pedro Alves
2017-05-03 22:46 ` [RFA 19/23] Replace do_restore_instream_cleanup with scoped_restore Tom Tromey
` (6 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
While working on the next patch in this series, I found that the
global in_user_command is not used. This patch removes it. (I didn't
think to check Insight until submitting this series; and it's not very
convenient to do so, so if someone has it checked out and could look
at it, that would be nice.)
2017-05-02 Tom Tromey <tom@tromey.com>
* top.h (in_user_command): Remove.
* top.c (in_user_command): Remove.
* cli/cli-script.c (do_restore_user_call_depth)
(execute_user_command): Update.
---
gdb/ChangeLog | 7 +++++++
gdb/cli/cli-script.c | 6 ------
gdb/top.c | 4 ----
gdb/top.h | 1 -
4 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9f75d1e..45c5db7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * top.h (in_user_command): Remove.
+ * top.c (in_user_command): Remove.
+ * cli/cli-script.c (do_restore_user_call_depth)
+ (execute_user_command): Update.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* valops.c (search_struct_method): Use std::vector.
* valarith.c (value_concat): Use std::vector.
* target.c (memory_xfer_partial): Use std::vector.
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 0aa1848..994f5c4 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -379,8 +379,6 @@ do_restore_user_call_depth (void * call_depth)
int *depth = (int *) call_depth;
(*depth)--;
- if ((*depth) == 0)
- in_user_command = 0;
}
@@ -411,10 +409,6 @@ execute_user_command (struct cmd_list_element *c, char *args)
make_cleanup (do_restore_instream_cleanup, ui->instream);
ui->instream = NULL;
- /* Also set the global in_user_command, so that NULL instream is
- not confused with Insight. */
- in_user_command = 1;
-
scoped_restore save_async = make_scoped_restore (¤t_ui->async, 0);
command_nest_depth++;
diff --git a/gdb/top.c b/gdb/top.c
index 652c55a..862016a 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -128,10 +128,6 @@ show_confirm (struct ui_file *file, int from_tty,
value);
}
-/* Flag to indicate whether a user defined command is currently running. */
-
-int in_user_command;
-
/* Current working directory. */
char *current_directory;
diff --git a/gdb/top.h b/gdb/top.h
index 452a381..5d7cb1f 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -218,7 +218,6 @@ extern void ui_unregister_input_event_handler (struct ui *ui);
/* From top.c. */
extern char *saved_command_line;
-extern int in_user_command;
extern int confirm;
extern char gdb_dirbuf[1024];
extern int inhibit_gdbinit;
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 16/23] Remove in_user_command
2017-05-03 22:46 ` [RFA 16/23] Remove in_user_command Tom Tromey
@ 2017-06-05 13:27 ` Pedro Alves
0 siblings, 0 replies; 59+ messages in thread
From: Pedro Alves @ 2017-06-05 13:27 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 05/03/2017 11:46 PM, Tom Tromey wrote:
> While working on the next patch in this series, I found that the
> global in_user_command is not used.
Looks like I removed the last (or only) user in:
268a799a454ce862f516ff2215290fae08eca7fa ("Make stdin be per UI")
https://sourceware.org/ml/gdb-patches/2016-05/msg00123.html
> This patch removes it. (I didn't
> think to check Insight until submitting this series; and it's not very
> convenient to do so, so if someone has it checked out and could look
> at it, that would be nice.)
Insight master is still based on a gdb from Jan 2016 so it still
has the use in the old input_from_terminal_p in top.c, but there's no
use in gdbtk/libgui. If insight really needs it when it rebases,
we can come up with something then.
>
> 2017-05-02 Tom Tromey <tom@tromey.com>
>
> * top.h (in_user_command): Remove.
> * top.c (in_user_command): Remove.
> * cli/cli-script.c (do_restore_user_call_depth)
> (execute_user_command): Update.
OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 59+ messages in thread
* [RFA 19/23] Replace do_restore_instream_cleanup with scoped_restore
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (16 preceding siblings ...)
2017-05-03 22:46 ` [RFA 16/23] Remove in_user_command Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-05 13:49 ` Pedro Alves
2017-05-03 22:46 ` [RFA 15/23] Use std::vector to avoid cleanups Tom Tromey
` (5 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes the users of do_restore_instream_cleanup to use a
scoped_restore instead. This patch is broken out because it warrants
some additional attention: in particular it's unclear to me whether
current_ui can change in the body of these functions -- but if it can,
then the cleanup would have modified a different UI's instream member.
2017-05-02 Tom Tromey <tom@tromey.com>
* top.h (do_restore_instream_cleanup): Remove.
* top.c (do_restore_instream_cleanup): Remove.
(read_command_file): Use scoped_restore.
* cli/cli-script.c (execute_user_command): Use scoped_restore.
---
gdb/ChangeLog | 7 +++++++
gdb/cli/cli-script.c | 6 ++----
gdb/top.c | 19 ++-----------------
gdb/top.h | 2 --
4 files changed, 11 insertions(+), 23 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a27feda..5e37c48 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * top.h (do_restore_instream_cleanup): Remove.
+ * top.c (do_restore_instream_cleanup): Remove.
+ (read_command_file): Use scoped_restore.
+ * cli/cli-script.c (execute_user_command): Use scoped_restore.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* cli/cli-script.c (execute_user_command)
(execute_control_command): Use scoped_restore.
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index c0e6716..b594630 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -377,7 +377,6 @@ execute_user_command (struct cmd_list_element *c, char *args)
{
struct ui *ui = current_ui;
struct command_line *cmdlines;
- struct cleanup *old_chain;
enum command_control_type ret;
static int user_call_depth = 0;
extern unsigned int max_user_call_depth;
@@ -396,8 +395,8 @@ execute_user_command (struct cmd_list_element *c, char *args)
/* Set the instream to 0, indicating execution of a
user-defined function. */
- old_chain = make_cleanup (do_restore_instream_cleanup, ui->instream);
- ui->instream = NULL;
+ scoped_restore restore_instream
+ = make_scoped_restore (&ui->instream, nullptr);
scoped_restore save_async = make_scoped_restore (¤t_ui->async, 0);
@@ -413,7 +412,6 @@ execute_user_command (struct cmd_list_element *c, char *args)
}
cmdlines = cmdlines->next;
}
- do_cleanups (old_chain);
}
/* This function is called every time GDB prints a prompt. It ensures
diff --git a/gdb/top.c b/gdb/top.c
index 862016a..6038366 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -443,27 +443,14 @@ quit_cover (void)
event-top.c into this file, top.c. */
/* static */ const char *source_file_name;
-/* Clean up on error during a "source" command (or execution of a
- user-defined command). */
-
-void
-do_restore_instream_cleanup (void *stream)
-{
- struct ui *ui = current_ui;
-
- /* Restore the previous input stream. */
- ui->instream = (FILE *) stream;
-}
-
/* Read commands from STREAM. */
void
read_command_file (FILE *stream)
{
struct ui *ui = current_ui;
- struct cleanup *cleanups;
- cleanups = make_cleanup (do_restore_instream_cleanup, ui->instream);
- ui->instream = stream;
+ scoped_restore save_instream
+ = make_scoped_restore (&ui->instream, stream);
/* Read commands from `instream' and execute them until end of file
or error reading instream. */
@@ -478,8 +465,6 @@ read_command_file (FILE *stream)
break;
command_handler (command);
}
-
- do_cleanups (cleanups);
}
\f
void (*pre_init_ui_hook) (void);
diff --git a/gdb/top.h b/gdb/top.h
index 5d7cb1f..4579889 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -290,8 +290,6 @@ extern void show_history (char *, int);
extern void set_verbose (char *, int, struct cmd_list_element *);
-extern void do_restore_instream_cleanup (void *stream);
-
extern char *handle_line_of_input (struct buffer *cmd_line_buffer,
char *rl, int repeat,
const char *annotation_suffix);
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 19/23] Replace do_restore_instream_cleanup with scoped_restore
2017-05-03 22:46 ` [RFA 19/23] Replace do_restore_instream_cleanup with scoped_restore Tom Tromey
@ 2017-06-05 13:49 ` Pedro Alves
0 siblings, 0 replies; 59+ messages in thread
From: Pedro Alves @ 2017-06-05 13:49 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 05/03/2017 11:46 PM, Tom Tromey wrote:
> This changes the users of do_restore_instream_cleanup to use a
> scoped_restore instead. This patch is broken out because it warrants
> some additional attention: in particular it's unclear to me whether
> current_ui can change in the body of these functions -- but if it can,
> then the cleanup would have modified a different UI's instream member.
>
The only places that change the current UI without restoring it
back are the top level event loop code that reacts to stdin input
and signal handlers (stdin_event_handler / invoke_async_signal_handlers).
Everything else that temporarily switches UI switches back before
returning, using scoped_restore. (This includes nested even loops like
wait_sync_command_done.)
> 2017-05-02 Tom Tromey <tom@tromey.com>
>
> * top.h (do_restore_instream_cleanup): Remove.
> * top.c (do_restore_instream_cleanup): Remove.
> (read_command_file): Use scoped_restore.
> * cli/cli-script.c (execute_user_command): Use scoped_restore.
OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 59+ messages in thread
* [RFA 15/23] Use std::vector to avoid cleanups
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (17 preceding siblings ...)
2017-05-03 22:46 ` [RFA 19/23] Replace do_restore_instream_cleanup with scoped_restore Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-02 19:22 ` Pedro Alves
2017-05-03 22:46 ` [RFA 07/23] Remove make_cleanup_fclose Tom Tromey
` (4 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This patch introduces std::vector in several spots in gdb that were
using xmalloc and a cleanup.
The simple_search_memory change warrants a bit of extra attention. It
was previously using malloc to try to handle memory allocation
failures. I don't know whether this was actually important.
2017-05-02 Tom Tromey <tom@tromey.com>
* valops.c (search_struct_method): Use std::vector.
* valarith.c (value_concat): Use std::vector.
* target.c (memory_xfer_partial): Use std::vector.
(simple_search_memory): Likewise.
* printcmd.c (find_string_backward): Use std::vector.
* mi/mi-main.c (mi_cmd_data_write_memory): Use std::vector.
* gcore.c (gcore_copy_callback): Use std::vector.
* elfread.c (elf_rel_plt_read): Use std::vector.
* cp-valprint.c (cp_print_value): Use std::vector.
* cli/cli-dump.c (restore_section_callback): Use std::vector.
---
gdb/ChangeLog | 13 +++++++++++++
gdb/cli/cli-dump.c | 11 +++--------
gdb/cp-valprint.c | 11 +++--------
gdb/elfread.c | 22 +++++++---------------
gdb/gcore.c | 12 ++++--------
gdb/mi/mi-main.c | 11 +++--------
gdb/printcmd.c | 10 +++-------
gdb/target.c | 36 ++++++++++--------------------------
gdb/valarith.c | 28 ++++++++++------------------
gdb/valops.c | 10 +++-------
10 files changed, 59 insertions(+), 105 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 03e733b..9f75d1e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,18 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * valops.c (search_struct_method): Use std::vector.
+ * valarith.c (value_concat): Use std::vector.
+ * target.c (memory_xfer_partial): Use std::vector.
+ (simple_search_memory): Likewise.
+ * printcmd.c (find_string_backward): Use std::vector.
+ * mi/mi-main.c (mi_cmd_data_write_memory): Use std::vector.
+ * gcore.c (gcore_copy_callback): Use std::vector.
+ * elfread.c (elf_rel_plt_read): Use std::vector.
+ * cp-valprint.c (cp_print_value): Use std::vector.
+ * cli/cli-dump.c (restore_section_callback): Use std::vector.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* jit.c (jit_reader_load_command): Use unique_xmalloc_ptr.
2017-05-02 Tom Tromey <tom@tromey.com>
diff --git a/gdb/cli/cli-dump.c b/gdb/cli/cli-dump.c
index 613ac1e..dfe99cd 100644
--- a/gdb/cli/cli-dump.c
+++ b/gdb/cli/cli-dump.c
@@ -434,8 +434,6 @@ restore_section_callback (bfd *ibfd, asection *isec, void *args)
bfd_vma sec_end = sec_start + size;
bfd_size_type sec_offset = 0;
bfd_size_type sec_load_count = size;
- struct cleanup *old_chain;
- gdb_byte *buf;
int ret;
/* Ignore non-loadable sections, eg. from elf files. */
@@ -463,9 +461,8 @@ restore_section_callback (bfd *ibfd, asection *isec, void *args)
sec_load_count -= sec_end - data->load_end;
/* Get the data. */
- buf = (gdb_byte *) xmalloc (size);
- old_chain = make_cleanup (xfree, buf);
- if (!bfd_get_section_contents (ibfd, isec, buf, 0, size))
+ std::vector<gdb_byte> buf (size);
+ if (!bfd_get_section_contents (ibfd, isec, buf.data (), 0, size))
error (_("Failed to read bfd file %s: '%s'."), bfd_get_filename (ibfd),
bfd_errmsg (bfd_get_error ()));
@@ -487,11 +484,9 @@ restore_section_callback (bfd *ibfd, asection *isec, void *args)
/* Write the data. */
ret = target_write_memory (sec_start + sec_offset + data->load_offset,
- buf + sec_offset, sec_load_count);
+ &buf[sec_offset], sec_load_count);
if (ret != 0)
warning (_("restore: memory write failed (%s)."), safe_strerror (ret));
- do_cleanups (old_chain);
- return;
}
static void
diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index 73fe03d..61507bd 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -534,23 +534,18 @@ cp_print_value (struct type *type, struct type *real_type,
if ((boffset + offset) < 0
|| (boffset + offset) >= TYPE_LENGTH (real_type))
{
- gdb_byte *buf;
- struct cleanup *back_to;
+ std::vector<gdb_byte> buf (TYPE_LENGTH (baseclass));
- buf = (gdb_byte *) xmalloc (TYPE_LENGTH (baseclass));
- back_to = make_cleanup (xfree, buf);
-
- if (target_read_memory (address + boffset, buf,
+ if (target_read_memory (address + boffset, buf.data (),
TYPE_LENGTH (baseclass)) != 0)
skip = 1;
base_val = value_from_contents_and_address (baseclass,
- buf,
+ buf.data (),
address + boffset);
baseclass = value_type (base_val);
thisoffset = 0;
boffset = 0;
thistype = baseclass;
- do_cleanups (back_to);
}
else
{
diff --git a/gdb/elfread.c b/gdb/elfread.c
index fba2026..a292fa8 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -540,9 +540,6 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
asection *plt, *relplt, *got_plt;
int plt_elf_idx;
bfd_size_type reloc_count, reloc;
- char *string_buffer = NULL;
- size_t string_buffer_size = 0;
- struct cleanup *back_to;
struct gdbarch *gdbarch = get_objfile_arch (objfile);
struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
size_t ptr_size = TYPE_LENGTH (ptr_type);
@@ -576,7 +573,7 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
if (! bed->s->slurp_reloc_table (obfd, relplt, dyn_symbol_table, TRUE))
return;
- back_to = make_cleanup (free_current_contents, &string_buffer);
+ std::vector<char> string_buffer;
reloc_count = relplt->size / elf_section_data (relplt)->this_hdr.sh_entsize;
for (reloc = 0; reloc < reloc_count; reloc++)
@@ -584,6 +581,7 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
const char *name;
struct minimal_symbol *msym;
CORE_ADDR address;
+ const char *got_suffix = SYMBOL_GOT_PLT_SUFFIX;
const size_t got_suffix_len = strlen (SYMBOL_GOT_PLT_SUFFIX);
size_t name_len;
@@ -601,24 +599,18 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
OBJFILE the symbol is undefined and the objfile having NAME defined
may not yet have been loaded. */
- if (string_buffer_size < name_len + got_suffix_len + 1)
- {
- string_buffer_size = 2 * (name_len + got_suffix_len);
- string_buffer = (char *) xrealloc (string_buffer, string_buffer_size);
- }
- memcpy (string_buffer, name, name_len);
- memcpy (&string_buffer[name_len], SYMBOL_GOT_PLT_SUFFIX,
- got_suffix_len + 1);
+ string_buffer.clear ();
+ string_buffer.insert (string_buffer.end (), name, name + name_len);
+ string_buffer.insert (string_buffer.end (), got_suffix,
+ got_suffix + got_suffix_len + 1);
- msym = record_minimal_symbol (reader, string_buffer,
+ msym = record_minimal_symbol (reader, string_buffer.data (),
name_len + got_suffix_len,
true, address, mst_slot_got_plt, got_plt,
objfile);
if (msym)
SET_MSYMBOL_SIZE (msym, ptr_size);
}
-
- do_cleanups (back_to);
}
/* The data pointer is htab_t for gnu_ifunc_record_cache_unchecked. */
diff --git a/gdb/gcore.c b/gdb/gcore.c
index c32d2ff..d8cbbdc 100644
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -548,8 +548,6 @@ gcore_copy_callback (bfd *obfd, asection *osec, void *ignored)
{
bfd_size_type size, total_size = bfd_section_size (obfd, osec);
file_ptr offset = 0;
- struct cleanup *old_chain = NULL;
- gdb_byte *memhunk;
/* Read-only sections are marked; we don't have to copy their contents. */
if ((bfd_get_section_flags (obfd, osec) & SEC_LOAD) == 0)
@@ -560,8 +558,7 @@ gcore_copy_callback (bfd *obfd, asection *osec, void *ignored)
return;
size = std::min (total_size, (bfd_size_type) MAX_COPY_BYTES);
- memhunk = (gdb_byte *) xmalloc (size);
- old_chain = make_cleanup (xfree, memhunk);
+ std::vector<gdb_byte> memhunk (size);
while (total_size > 0)
{
@@ -569,7 +566,7 @@ gcore_copy_callback (bfd *obfd, asection *osec, void *ignored)
size = total_size;
if (target_read_memory (bfd_section_vma (obfd, osec) + offset,
- memhunk, size) != 0)
+ memhunk.data (), size) != 0)
{
warning (_("Memory read failed for corefile "
"section, %s bytes at %s."),
@@ -577,7 +574,8 @@ gcore_copy_callback (bfd *obfd, asection *osec, void *ignored)
paddress (target_gdbarch (), bfd_section_vma (obfd, osec)));
break;
}
- if (!bfd_set_section_contents (obfd, osec, memhunk, offset, size))
+ if (!bfd_set_section_contents (obfd, osec, memhunk.data (),
+ offset, size))
{
warning (_("Failed to write corefile contents (%s)."),
bfd_errmsg (bfd_get_error ()));
@@ -587,8 +585,6 @@ gcore_copy_callback (bfd *obfd, asection *osec, void *ignored)
total_size -= size;
offset += size;
}
-
- do_cleanups (old_chain); /* Frees MEMHUNK. */
}
static int
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index aa44876..f4adda6 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1658,8 +1658,6 @@ mi_cmd_data_write_memory (const char *command, char **argv, int argc)
/* FIXME: ezannoni 2000-02-17 LONGEST could possibly not be big
enough when using a compiler other than GCC. */
LONGEST value;
- gdb_byte *buffer;
- struct cleanup *old_chain;
long offset = 0;
int oind = 0;
char *oarg;
@@ -1706,13 +1704,10 @@ mi_cmd_data_write_memory (const char *command, char **argv, int argc)
/* Get the value as a number. */
value = parse_and_eval_address (argv[3]);
/* Get the value into an array. */
- buffer = (gdb_byte *) xmalloc (word_size);
- old_chain = make_cleanup (xfree, buffer);
- store_signed_integer (buffer, word_size, byte_order, value);
+ std::vector<gdb_byte> buffer (word_size);
+ store_signed_integer (buffer.data (), word_size, byte_order, value);
/* Write it down to memory. */
- write_memory_with_notification (addr, buffer, word_size);
- /* Free the buffer. */
- do_cleanups (old_chain);
+ write_memory_with_notification (addr, buffer.data (), word_size);
}
/* Implementation of the -data-write-memory-bytes command.
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 02d6e1c..e72f02c 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -948,8 +948,6 @@ find_string_backward (struct gdbarch *gdbarch,
int *strings_counted)
{
const int chunk_size = 0x20;
- gdb_byte *buffer = NULL;
- struct cleanup *cleanup = NULL;
int read_error = 0;
int chars_read = 0;
int chars_to_read = chunk_size;
@@ -958,14 +956,13 @@ find_string_backward (struct gdbarch *gdbarch,
CORE_ADDR string_start_addr = addr;
gdb_assert (char_size == 1 || char_size == 2 || char_size == 4);
- buffer = (gdb_byte *) xmalloc (chars_to_read * char_size);
- cleanup = make_cleanup (xfree, buffer);
+ std::vector<gdb_byte> buffer (chars_to_read * char_size);
while (count > 0 && read_error == 0)
{
int i;
addr -= chars_to_read * char_size;
- chars_read = read_memory_backward (gdbarch, addr, buffer,
+ chars_read = read_memory_backward (gdbarch, addr, buffer.data (),
chars_to_read * char_size);
chars_read /= char_size;
read_error = (chars_read == chars_to_read) ? 0 : 1;
@@ -974,7 +971,7 @@ find_string_backward (struct gdbarch *gdbarch,
{
int offset = (chars_to_read - i - 1) * char_size;
- if (integer_is_zero (buffer + offset, char_size)
+ if (integer_is_zero (&buffer[offset], char_size)
|| chars_counted == options->print_max)
{
/* Found '\0' or reached print_max. As OFFSET is the offset to
@@ -997,7 +994,6 @@ find_string_backward (struct gdbarch *gdbarch,
string_start_addr -= chars_counted * char_size;
}
- do_cleanups (cleanup);
return string_start_addr;
}
diff --git a/gdb/target.c b/gdb/target.c
index e526bcc..660c2ef 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1284,9 +1284,6 @@ memory_xfer_partial (struct target_ops *ops, enum target_object object,
}
else
{
- gdb_byte *buf;
- struct cleanup *old_chain;
-
/* A large write request is likely to be partially satisfied
by memory_xfer_partial_1. We will continually malloc
and free a copy of the entire write request for breakpoint
@@ -1295,15 +1292,10 @@ memory_xfer_partial (struct target_ops *ops, enum target_object object,
to mitigate this. */
len = std::min (ops->to_get_memory_xfer_limit (ops), len);
- buf = (gdb_byte *) xmalloc (len);
- old_chain = make_cleanup (xfree, buf);
- memcpy (buf, writebuf, len);
-
- breakpoint_xfer_memory (NULL, buf, writebuf, memaddr, len);
- res = memory_xfer_partial_1 (ops, object, NULL, buf, memaddr, len,
+ std::vector<gdb_byte> buf (writebuf, writebuf + len);
+ breakpoint_xfer_memory (NULL, buf.data (), writebuf, memaddr, len);
+ res = memory_xfer_partial_1 (ops, object, NULL, buf.data (), memaddr, len,
xfered_len);
-
- do_cleanups (old_chain);
}
return res;
@@ -2439,9 +2431,7 @@ simple_search_memory (struct target_ops *ops,
#define SEARCH_CHUNK_SIZE 16000
const unsigned chunk_size = SEARCH_CHUNK_SIZE;
/* Buffer to hold memory contents for searching. */
- gdb_byte *search_buf;
unsigned search_buf_size;
- struct cleanup *old_cleanups;
search_buf_size = chunk_size + pattern_len - 1;
@@ -2449,20 +2439,17 @@ simple_search_memory (struct target_ops *ops,
if (search_space_len < search_buf_size)
search_buf_size = search_space_len;
- search_buf = (gdb_byte *) malloc (search_buf_size);
- if (search_buf == NULL)
- error (_("Unable to allocate memory to perform the search."));
- old_cleanups = make_cleanup (free_current_contents, &search_buf);
+ std::vector<gdb_byte> search_buf (search_buf_size);
/* Prime the search buffer. */
if (target_read (ops, TARGET_OBJECT_MEMORY, NULL,
- search_buf, start_addr, search_buf_size) != search_buf_size)
+ search_buf.data (), start_addr, search_buf_size)
+ != search_buf_size)
{
warning (_("Unable to access %s bytes of target "
"memory at %s, halting search."),
pulongest (search_buf_size), hex_string (start_addr));
- do_cleanups (old_cleanups);
return -1;
}
@@ -2478,15 +2465,14 @@ simple_search_memory (struct target_ops *ops,
unsigned nr_search_bytes
= std::min (search_space_len, (ULONGEST) search_buf_size);
- found_ptr = (gdb_byte *) memmem (search_buf, nr_search_bytes,
+ found_ptr = (gdb_byte *) memmem (search_buf.data (), nr_search_bytes,
pattern, pattern_len);
if (found_ptr != NULL)
{
- CORE_ADDR found_addr = start_addr + (found_ptr - search_buf);
+ CORE_ADDR found_addr = start_addr + (found_ptr - search_buf.data ());
*found_addrp = found_addr;
- do_cleanups (old_cleanups);
return 1;
}
@@ -2507,20 +2493,19 @@ simple_search_memory (struct target_ops *ops,
/* Copy the trailing part of the previous iteration to the front
of the buffer for the next iteration. */
gdb_assert (keep_len == pattern_len - 1);
- memcpy (search_buf, search_buf + chunk_size, keep_len);
+ memcpy (&search_buf[0], &search_buf[chunk_size], keep_len);
nr_to_read = std::min (search_space_len - keep_len,
(ULONGEST) chunk_size);
if (target_read (ops, TARGET_OBJECT_MEMORY, NULL,
- search_buf + keep_len, read_addr,
+ &search_buf[keep_len], read_addr,
nr_to_read) != nr_to_read)
{
warning (_("Unable to access %s bytes of target "
"memory at %s, halting search."),
plongest (nr_to_read),
hex_string (read_addr));
- do_cleanups (old_cleanups);
return -1;
}
@@ -2530,7 +2515,6 @@ simple_search_memory (struct target_ops *ops,
/* Not found. */
- do_cleanups (old_cleanups);
return 0;
}
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 985233c..9724aca 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -697,12 +697,9 @@ value_concat (struct value *arg1, struct value *arg2)
if (TYPE_CODE (type2) == TYPE_CODE_STRING
|| TYPE_CODE (type2) == TYPE_CODE_CHAR)
{
- struct cleanup *back_to;
-
count = longest_to_int (value_as_long (inval1));
inval2len = TYPE_LENGTH (type2);
- ptr = (char *) xmalloc (count * inval2len);
- back_to = make_cleanup (xfree, ptr);
+ std::vector<char> ptr (count * inval2len);
if (TYPE_CODE (type2) == TYPE_CODE_CHAR)
{
char_type = type2;
@@ -711,7 +708,7 @@ value_concat (struct value *arg1, struct value *arg2)
value_contents (inval2));
for (idx = 0; idx < count; idx++)
{
- *(ptr + idx) = inchar;
+ ptr[idx] = inchar;
}
}
else
@@ -720,12 +717,11 @@ value_concat (struct value *arg1, struct value *arg2)
for (idx = 0; idx < count; idx++)
{
- memcpy (ptr + (idx * inval2len), value_contents (inval2),
+ memcpy (&ptr[idx * inval2len], value_contents (inval2),
inval2len);
}
}
- outval = value_string (ptr, count * inval2len, char_type);
- do_cleanups (back_to);
+ outval = value_string (ptr.data (), count * inval2len, char_type);
}
else if (TYPE_CODE (type2) == TYPE_CODE_BOOL)
{
@@ -739,8 +735,6 @@ value_concat (struct value *arg1, struct value *arg2)
else if (TYPE_CODE (type1) == TYPE_CODE_STRING
|| TYPE_CODE (type1) == TYPE_CODE_CHAR)
{
- struct cleanup *back_to;
-
/* We have two character strings to concatenate. */
if (TYPE_CODE (type2) != TYPE_CODE_STRING
&& TYPE_CODE (type2) != TYPE_CODE_CHAR)
@@ -749,31 +743,29 @@ value_concat (struct value *arg1, struct value *arg2)
}
inval1len = TYPE_LENGTH (type1);
inval2len = TYPE_LENGTH (type2);
- ptr = (char *) xmalloc (inval1len + inval2len);
- back_to = make_cleanup (xfree, ptr);
+ std::vector<char> ptr (inval1len + inval2len);
if (TYPE_CODE (type1) == TYPE_CODE_CHAR)
{
char_type = type1;
- *ptr = (char) unpack_long (type1, value_contents (inval1));
+ ptr[0] = (char) unpack_long (type1, value_contents (inval1));
}
else
{
char_type = TYPE_TARGET_TYPE (type1);
- memcpy (ptr, value_contents (inval1), inval1len);
+ memcpy (ptr.data (), value_contents (inval1), inval1len);
}
if (TYPE_CODE (type2) == TYPE_CODE_CHAR)
{
- *(ptr + inval1len) =
+ ptr[inval1len] =
(char) unpack_long (type2, value_contents (inval2));
}
else
{
- memcpy (ptr + inval1len, value_contents (inval2), inval2len);
+ memcpy (&ptr[inval1len], value_contents (inval2), inval2len);
}
- outval = value_string (ptr, inval1len + inval2len, char_type);
- do_cleanups (back_to);
+ outval = value_string (ptr.data (), inval1len + inval2len, char_type);
}
else if (TYPE_CODE (type1) == TYPE_CODE_BOOL)
{
diff --git a/gdb/valops.c b/gdb/valops.c
index 93ae6cf..69444d8 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2075,24 +2075,20 @@ search_struct_method (const char *name, struct value **arg1p,
if (offset < 0 || offset >= TYPE_LENGTH (type))
{
- gdb_byte *tmp;
- struct cleanup *back_to;
CORE_ADDR address;
- tmp = (gdb_byte *) xmalloc (TYPE_LENGTH (baseclass));
- back_to = make_cleanup (xfree, tmp);
+ std::vector<gdb_byte> tmp (TYPE_LENGTH (baseclass));
address = value_address (*arg1p);
if (target_read_memory (address + offset,
- tmp, TYPE_LENGTH (baseclass)) != 0)
+ tmp.data (), TYPE_LENGTH (baseclass)) != 0)
error (_("virtual baseclass botch"));
base_val = value_from_contents_and_address (baseclass,
- tmp,
+ tmp.data (),
address + offset);
base_valaddr = value_contents_for_printing (base_val);
this_offset = 0;
- do_cleanups (back_to);
}
else
{
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 15/23] Use std::vector to avoid cleanups
2017-05-03 22:46 ` [RFA 15/23] Use std::vector to avoid cleanups Tom Tromey
@ 2017-06-02 19:22 ` Pedro Alves
2017-07-23 16:06 ` Tom Tromey
0 siblings, 1 reply; 59+ messages in thread
From: Pedro Alves @ 2017-06-02 19:22 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 05/03/2017 11:46 PM, Tom Tromey wrote:
> This patch introduces std::vector in several spots in gdb that were
> using xmalloc and a cleanup.
>
> The simple_search_memory change warrants a bit of extra attention. It
> was previously using malloc to try to handle memory allocation
> failures. I don't know whether this was actually important.
(
It still pains me a little to see std::vector used when we're going to
fill in the data anyway, because std::vector value/zero-initializes,
which is pointless in these cases. :-) I think I'll need to accept
that people dislike the idea of unique_ptr<gdb_byte[]>, so I'll stop
mentioning that, though at some point I think I may propose an
allocator that default-initializes, and then I'll nag people
to use _that_, like:
typedef std::vector<gdb_byte, default_init_allocator> gdb_byte_vec;
gdb_byte_vec buf (size);
)
> @@ -601,24 +599,18 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
> OBJFILE the symbol is undefined and the objfile having NAME defined
> may not yet have been loaded. */
>
> - if (string_buffer_size < name_len + got_suffix_len + 1)
> - {
> - string_buffer_size = 2 * (name_len + got_suffix_len);
> - string_buffer = (char *) xrealloc (string_buffer, string_buffer_size);
> - }
> - memcpy (string_buffer, name, name_len);
> - memcpy (&string_buffer[name_len], SYMBOL_GOT_PLT_SUFFIX,
> - got_suffix_len + 1);
> + string_buffer.clear ();
> + string_buffer.insert (string_buffer.end (), name, name + name_len);
> + string_buffer.insert (string_buffer.end (), got_suffix,
> + got_suffix + got_suffix_len + 1);
So the patch all looks good to me, but it seems to me that in this
particular case, making this buffer be a std::string would clarify
the code:
string_buffer.assign (name, name_len);
string_buffer.append (got_suffix, got_suffix_len);
>
> - msym = record_minimal_symbol (reader, string_buffer,
> + msym = record_minimal_symbol (reader, string_buffer.data (),
> name_len + got_suffix_len,
and here maybe use string_buffer.size ()
(std::string is guaranteed to be null terminated.)
> true, address, mst_slot_got_plt, got_plt,
> objfile);
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 15/23] Use std::vector to avoid cleanups
2017-06-02 19:22 ` Pedro Alves
@ 2017-07-23 16:06 ` Tom Tromey
0 siblings, 0 replies; 59+ messages in thread
From: Tom Tromey @ 2017-07-23 16:06 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> It still pains me a little to see std::vector used when we're going to
Pedro> fill in the data anyway, because std::vector value/zero-initializes,
Pedro> which is pointless in these cases. :-) I think I'll need to accept
Pedro> that people dislike the idea of unique_ptr<gdb_byte[]>, so I'll stop
Pedro> mentioning that, though at some point I think I may propose an
Pedro> allocator that default-initializes, and then I'll nag people
Pedro> to use _that_, like:
I've updated the patch to use this where appppropriate.
Pedro> So the patch all looks good to me, but it seems to me that in this
Pedro> particular case, making this buffer be a std::string would clarify
Pedro> the code:
[...]
I made this change as well.
Tom
^ permalink raw reply [flat|nested] 59+ messages in thread
* [RFA 07/23] Remove make_cleanup_fclose
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (18 preceding siblings ...)
2017-05-03 22:46 ` [RFA 15/23] Use std::vector to avoid cleanups Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-02 17:32 ` Pedro Alves
2017-05-03 22:46 ` [RFA 05/23] Use gdb_file_up in source.c Tom Tromey
` (3 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
After the preceding patches, make_cleanup_fclose is no longer used, so
remove it.
2017-05-02 Tom Tromey <tom@tromey.com>
* utils.h (make_cleanup_fclose): Remove.
* utils.c (do_fclose_cleanup, make_cleanup_fclose): Remove.
---
gdb/ChangeLog | 5 +++++
gdb/utils.c | 18 ------------------
gdb/utils.h | 2 --
3 files changed, 5 insertions(+), 20 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1dad15be..2f1219f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * utils.h (make_cleanup_fclose): Remove.
+ * utils.c (do_fclose_cleanup, make_cleanup_fclose): Remove.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* top.c (open_terminal_stream): Return gdb_file_up.
(new_ui_command): Update.
diff --git a/gdb/utils.c b/gdb/utils.c
index b4332f8..bffce78 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -151,24 +151,6 @@ make_cleanup_freeargv (char **arg)
return make_cleanup (do_freeargv, arg);
}
-/* Helper function which does the work for make_cleanup_fclose. */
-
-static void
-do_fclose_cleanup (void *arg)
-{
- FILE *file = (FILE *) arg;
-
- fclose (file);
-}
-
-/* Return a new cleanup that closes FILE. */
-
-struct cleanup *
-make_cleanup_fclose (FILE *file)
-{
- return make_cleanup (do_fclose_cleanup, file);
-}
-
/* Helper function which does the work for make_cleanup_obstack_free. */
static void
diff --git a/gdb/utils.h b/gdb/utils.h
index f3e8007..7afb3c5 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -76,8 +76,6 @@ extern struct cleanup *(make_cleanup_free_section_addr_info
/* For make_cleanup_close see common/filestuff.h. */
-extern struct cleanup *make_cleanup_fclose (FILE *file);
-
struct obstack;
extern struct cleanup *make_cleanup_obstack_free (struct obstack *obstack);
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* [RFA 05/23] Use gdb_file_up in source.c
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (19 preceding siblings ...)
2017-05-03 22:46 ` [RFA 07/23] Remove make_cleanup_fclose Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-02 17:27 ` Pedro Alves
2017-05-03 22:46 ` [RFA 06/23] Change open_terminal_stream to return a gdb_file_up Tom Tromey
` (2 subsequent siblings)
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes some functions in source.c to use gdb_file_up.
2017-05-02 Tom Tromey <tom@tromey.com>
* source.c (print_source_lines_base, forward_search_command)
(reverse_search_command): Use gdb_file_up.
---
gdb/ChangeLog | 5 +++++
gdb/source.c | 45 ++++++++++++++++-----------------------------
2 files changed, 21 insertions(+), 29 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 799b727..9324d44 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * source.c (print_source_lines_base, forward_search_command)
+ (reverse_search_command): Use gdb_file_up.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* fbsd-nat.c (fbsd_find_memory_regions): Update.
2017-05-02 Tom Tromey <tom@tromey.com>
diff --git a/gdb/source.c b/gdb/source.c
index 8926e54..4cc862c 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1353,9 +1353,7 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
int c;
int desc;
int noprint = 0;
- FILE *stream;
int nlines = stopline - line;
- struct cleanup *cleanup;
struct ui_out *uiout = current_uiout;
/* Regardless of whether we can open the file, set current_source_symtab. */
@@ -1448,15 +1446,14 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
perror_with_name (symtab_to_filename_for_display (s));
}
- stream = fdopen (desc, FDOPEN_MODE);
- clearerr (stream);
- cleanup = make_cleanup_fclose (stream);
+ gdb_file_up stream (fdopen (desc, FDOPEN_MODE));
+ clearerr (stream.get ());
while (nlines-- > 0)
{
char buf[20];
- c = fgetc (stream);
+ c = fgetc (stream.get ());
if (c == EOF)
break;
last_line_listed = current_source_line;
@@ -1479,12 +1476,12 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
else if (c == '\r')
{
/* Skip a \r character, but only before a \n. */
- int c1 = fgetc (stream);
+ int c1 = fgetc (stream.get ());
if (c1 != '\n')
printf_filtered ("^%c", c + 0100);
if (c1 != EOF)
- ungetc (c1, stream);
+ ungetc (c1, stream.get ());
}
else
{
@@ -1492,10 +1489,8 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
uiout->text (buf);
}
}
- while (c != '\n' && (c = fgetc (stream)) >= 0);
+ while (c != '\n' && (c = fgetc (stream.get ())) >= 0);
}
-
- do_cleanups (cleanup);
}
\f
/* Show source lines from the file of symtab S, starting with line
@@ -1630,7 +1625,6 @@ forward_search_command (char *regex, int from_tty)
{
int c;
int desc;
- FILE *stream;
int line;
char *msg;
struct cleanup *cleanups;
@@ -1659,9 +1653,8 @@ forward_search_command (char *regex, int from_tty)
perror_with_name (symtab_to_filename_for_display (current_source_symtab));
discard_cleanups (cleanups);
- stream = fdopen (desc, FDOPEN_MODE);
- clearerr (stream);
- cleanups = make_cleanup_fclose (stream);
+ gdb_file_up stream (fdopen (desc, FDOPEN_MODE));
+ clearerr (stream.get ());
while (1)
{
static char *buf = NULL;
@@ -1672,7 +1665,7 @@ forward_search_command (char *regex, int from_tty)
buf = (char *) xmalloc (cursize);
p = buf;
- c = fgetc (stream);
+ c = fgetc (stream.get ());
if (c == EOF)
break;
do
@@ -1686,7 +1679,7 @@ forward_search_command (char *regex, int from_tty)
cursize = newsize;
}
}
- while (c != '\n' && (c = fgetc (stream)) >= 0);
+ while (c != '\n' && (c = fgetc (stream.get ())) >= 0);
/* Remove the \r, if any, at the end of the line, otherwise
regular expressions that end with $ or \n won't work. */
@@ -1701,7 +1694,6 @@ forward_search_command (char *regex, int from_tty)
if (re_exec (buf) > 0)
{
/* Match! */
- do_cleanups (cleanups);
print_source_lines (current_source_symtab, line, line + 1, 0);
set_internalvar_integer (lookup_internalvar ("_"), line);
current_source_line = std::max (line - lines_to_list / 2, 1);
@@ -1711,7 +1703,6 @@ forward_search_command (char *regex, int from_tty)
}
printf_filtered (_("Expression not found\n"));
- do_cleanups (cleanups);
}
static void
@@ -1719,7 +1710,6 @@ reverse_search_command (char *regex, int from_tty)
{
int c;
int desc;
- FILE *stream;
int line;
char *msg;
struct cleanup *cleanups;
@@ -1748,23 +1738,22 @@ reverse_search_command (char *regex, int from_tty)
perror_with_name (symtab_to_filename_for_display (current_source_symtab));
discard_cleanups (cleanups);
- stream = fdopen (desc, FDOPEN_MODE);
- clearerr (stream);
- cleanups = make_cleanup_fclose (stream);
+ gdb_file_up stream (fdopen (desc, FDOPEN_MODE));
+ clearerr (stream.get ());
while (line > 1)
{
/* FIXME!!! We walk right off the end of buf if we get a long line!!! */
char buf[4096]; /* Should be reasonable??? */
char *p = buf;
- c = fgetc (stream);
+ c = fgetc (stream.get ());
if (c == EOF)
break;
do
{
*p++ = c;
}
- while (c != '\n' && (c = fgetc (stream)) >= 0);
+ while (c != '\n' && (c = fgetc (stream.get ())) >= 0);
/* Remove the \r, if any, at the end of the line, otherwise
regular expressions that end with $ or \n won't work. */
@@ -1779,25 +1768,23 @@ reverse_search_command (char *regex, int from_tty)
if (re_exec (buf) > 0)
{
/* Match! */
- do_cleanups (cleanups);
print_source_lines (current_source_symtab, line, line + 1, 0);
set_internalvar_integer (lookup_internalvar ("_"), line);
current_source_line = std::max (line - lines_to_list / 2, 1);
return;
}
line--;
- if (fseek (stream, current_source_symtab->line_charpos[line - 1], 0) < 0)
+ if (fseek (stream.get (),
+ current_source_symtab->line_charpos[line - 1], 0) < 0)
{
const char *filename;
- do_cleanups (cleanups);
filename = symtab_to_filename_for_display (current_source_symtab);
perror_with_name (filename);
}
}
printf_filtered (_("Expression not found\n"));
- do_cleanups (cleanups);
return;
}
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* [RFA 06/23] Change open_terminal_stream to return a gdb_file_up
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (20 preceding siblings ...)
2017-05-03 22:46 ` [RFA 05/23] Use gdb_file_up in source.c Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-02 17:31 ` Pedro Alves
2017-05-03 22:46 ` [RFA 18/23] Use a scoped_restore for command_nest_depth Tom Tromey
2017-05-29 17:31 ` [RFA 00/23] More miscellaneous C++-ification Tom Tromey
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes open_terminal_stream to return a gdb_file_up, eliminating
another use of make_cleanup_fclose. Arguably perhaps new_ui should
take ownership of the files using a move, but there is at least one
spot where this isn't appropriate (or at least not currently done), so
I elected to use a more minimal approach.
2017-05-02 Tom Tromey <tom@tromey.com>
* top.c (open_terminal_stream): Return gdb_file_up.
(new_ui_command): Update.
---
gdb/ChangeLog | 5 +++++
gdb/top.c | 24 ++++++++++++------------
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9324d44..1dad15be 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * top.c (open_terminal_stream): Return gdb_file_up.
+ (new_ui_command): Update.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* source.c (print_source_lines_base, forward_search_command)
(reverse_search_command): Use gdb_file_up.
diff --git a/gdb/top.c b/gdb/top.c
index 3de8a26..652c55a 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -345,16 +345,16 @@ make_delete_ui_cleanup (struct ui *ui)
/* Open file named NAME for read/write, making sure not to make it the
controlling terminal. */
-static FILE *
+static gdb_file_up
open_terminal_stream (const char *name)
{
int fd;
- fd = open (name, O_RDWR | O_NOCTTY);
+ fd = gdb_open_cloexec (name, O_RDWR | O_NOCTTY, 0);
if (fd < 0)
perror_with_name (_("opening terminal failed"));
- return fdopen (fd, "w+");
+ return gdb_file_up (fdopen (fd, "w+"));
}
/* Implementation of the "new-ui" command. */
@@ -364,7 +364,7 @@ new_ui_command (char *args, int from_tty)
{
struct ui *ui;
struct interp *interp;
- FILE *stream[3] = { NULL, NULL, NULL };
+ gdb_file_up stream[3];
int i;
int res;
int argc;
@@ -389,18 +389,13 @@ new_ui_command (char *args, int from_tty)
{
scoped_restore save_ui = make_scoped_restore (¤t_ui);
- failure_chain = make_cleanup (null_cleanup, NULL);
-
/* Open specified terminal, once for each of
stdin/stdout/stderr. */
for (i = 0; i < 3; i++)
- {
- stream[i] = open_terminal_stream (tty_name);
- make_cleanup_fclose (stream[i]);
- }
+ stream[i] = open_terminal_stream (tty_name);
- ui = new_ui (stream[0], stream[1], stream[2]);
- make_cleanup (delete_ui_cleanup, ui);
+ ui = new_ui (stream[0].get (), stream[1].get (), stream[2].get ());
+ failure_chain = make_cleanup (delete_ui_cleanup, ui);
ui->async = 1;
@@ -410,6 +405,11 @@ new_ui_command (char *args, int from_tty)
interp_pre_command_loop (top_level_interpreter ());
+ /* Make sure the files are not closed. */
+ stream[0].release ();
+ stream[1].release ();
+ stream[2].release ();
+
discard_cleanups (failure_chain);
/* This restores the previous UI and frees argv. */
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* [RFA 18/23] Use a scoped_restore for command_nest_depth
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (21 preceding siblings ...)
2017-05-03 22:46 ` [RFA 06/23] Change open_terminal_stream to return a gdb_file_up Tom Tromey
@ 2017-05-03 22:46 ` Tom Tromey
2017-06-05 13:38 ` Pedro Alves
2017-05-29 17:31 ` [RFA 00/23] More miscellaneous C++-ification Tom Tromey
23 siblings, 1 reply; 59+ messages in thread
From: Tom Tromey @ 2017-05-03 22:46 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes a couple of places to use a scoped_restore when
manipulating command_nest_depth.
2017-05-02 Tom Tromey <tom@tromey.com>
* cli/cli-script.c (execute_user_command)
(execute_control_command): Use scoped_restore.
---
gdb/ChangeLog | 5 +++++
gdb/cli/cli-script.c | 12 ++++++------
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 873b404..a27feda 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
2017-05-02 Tom Tromey <tom@tromey.com>
+ * cli/cli-script.c (execute_user_command)
+ (execute_control_command): Use scoped_restore.
+
+2017-05-02 Tom Tromey <tom@tromey.com>
+
* cli/cli-script.c (do_restore_user_call_depth): Remove.
(execute_user_command): Use scoped_restore.
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 7f90d27..c0e6716 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -401,7 +401,8 @@ execute_user_command (struct cmd_list_element *c, char *args)
scoped_restore save_async = make_scoped_restore (¤t_ui->async, 0);
- command_nest_depth++;
+ scoped_restore save_nesting
+ = make_scoped_restore (&command_nest_depth, command_nest_depth + 1);
while (cmdlines)
{
ret = execute_control_command (cmdlines);
@@ -412,7 +413,6 @@ execute_user_command (struct cmd_list_element *c, char *args)
}
cmdlines = cmdlines->next;
}
- command_nest_depth--;
do_cleanups (old_chain);
}
@@ -532,9 +532,9 @@ execute_control_command (struct command_line *cmd)
current = *cmd->body_list;
while (current)
{
- command_nest_depth++;
+ scoped_restore save_nesting
+ = make_scoped_restore (&command_nest_depth, command_nest_depth + 1);
ret = execute_control_command (current);
- command_nest_depth--;
/* If we got an error, or a "break" command, then stop
looping. */
@@ -591,9 +591,9 @@ execute_control_command (struct command_line *cmd)
/* Execute commands in the given arm. */
while (current)
{
- command_nest_depth++;
+ scoped_restore save_nesting
+ = make_scoped_restore (&command_nest_depth, command_nest_depth + 1);
ret = execute_control_command (current);
- command_nest_depth--;
/* If we got an error, get out. */
if (ret != simple_control)
--
2.9.3
^ permalink raw reply [flat|nested] 59+ messages in thread* Re: [RFA 00/23] More miscellaneous C++-ification
2017-05-03 22:46 [RFA 00/23] More miscellaneous C++-ification Tom Tromey
` (22 preceding siblings ...)
2017-05-03 22:46 ` [RFA 18/23] Use a scoped_restore for command_nest_depth Tom Tromey
@ 2017-05-29 17:31 ` Tom Tromey
23 siblings, 0 replies; 59+ messages in thread
From: Tom Tromey @ 2017-05-29 17:31 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Tom> This is a series of random C++-ification. There's no real rhyme or
Tom> reason to my approach, except that one goal was to remove the
Tom> remaining cleanups from the Python code.
Ping for this series.
^ permalink raw reply [flat|nested] 59+ messages in thread