This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Use gdb::unique_xmalloc_ptr when calling tilde_expand


Hi Tom,

On 2017-08-03 23:43, Tom Tromey wrote:
This patch changes most sites calling tilde_expand to use
gdb::unique_xmalloc_ptr, rather than a cleanup.  It also changes
scan_expression_with_cleanup to return a unique pointer, because the
patch was already touching code in that area.

The patch looks good to me. I noted two formatting nits (that were present before this patch), could you fix them before pushing?

@@ -224,14 +210,12 @@ dump_memory_to_file (const char *cmd, const char
*mode, const char *file_format)
   /* Have everything.  Open/write the data.  */
   if (file_format == NULL || strcmp (file_format, "binary") == 0)
     {
-      dump_binary_file (filename, mode, buf.data (), count);
+      dump_binary_file (filename.get (), mode, buf.data (), count);
     }
   else
     {
- dump_bfd_file (filename, mode, file_format, lo, buf.data (), count);
+      dump_bfd_file (filename.get (), mode, file_format, lo, buf.data
(), count);
     }

Remove these unnecessary curly braces.

@@ -589,18 +568,18 @@ restore_command (char *args_in, int from_tty)

   if (info_verbose)
printf_filtered ("Restore file %s offset 0x%lx start 0x%lx end 0x%lx\n",
-		     filename, (unsigned long) data.load_offset,
+		     filename.get (), (unsigned long) data.load_offset,
 		     (unsigned long) data.load_start,
 		     (unsigned long) data.load_end);

   if (binary_flag)
     {
-      restore_binary_file (filename, &data);
+      restore_binary_file (filename.get (), &data);
     }

Same here.


diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index fb4283f..37bd96a 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -423,7 +423,6 @@ static void
 tfile_open (const char *arg, int from_tty)
 {
   char *temp;
-  struct cleanup *old_chain;
   int flags;
   int scratch_chan;
   char header[TRACE_HEADER_SIZE];
@@ -433,34 +432,27 @@ tfile_open (const char *arg, int from_tty)
   struct trace_status *ts;
   struct uploaded_tp *uploaded_tps = NULL;
   struct uploaded_tsv *uploaded_tsvs = NULL;
-  char *filename;

   target_preopen (from_tty);
   if (!arg)
     error (_("No trace file specified."));

-  filename = tilde_expand (arg);
-  if (!IS_ABSOLUTE_PATH(filename))
-    {
-      temp = concat (current_directory, "/", filename, (char *) NULL);
-      xfree (filename);
-      filename = temp;
-    }
-
-  old_chain = make_cleanup (xfree, filename);
+  gdb::unique_xmalloc_ptr<char> filename (tilde_expand (arg));
+  if (!IS_ABSOLUTE_PATH (filename.get ()))
+    filename.reset (concat (current_directory, "/", filename.get (),
+			    (char *) NULL));

   flags = O_BINARY | O_LARGEFILE;
   flags |= O_RDONLY;
-  scratch_chan = gdb_open_cloexec (filename, flags, 0);
+  scratch_chan = gdb_open_cloexec (filename.get (), flags, 0);
   if (scratch_chan < 0)
-    perror_with_name (filename);
+    perror_with_name (filename.get ());

/* Looks semi-reasonable. Toss the old trace file and work on the new. */

-  discard_cleanups (old_chain);	/* Don't free filename any more.  */
   unpush_target (&tfile_ops);

-  trace_filename = xstrdup (filename);
+  trace_filename = filename.release ();

Hmm, the old code discarded the cleanup and did an xstrdup, that seems like a leak that you fixed :)

Thanks,

Simon


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]