public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb: make target_ops::make_corefile_notes return a unique ptr
@ 2020-10-21 19:21 Simon Marchi
  2020-10-22 16:51 ` Tom Tromey
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2020-10-21 19:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

Since we converted gdbarch_make_corefile_notes to returning a
gdb::unique_xmalloc_ptr, I figured it would make sense to converted
target_ops::make_corefile_notes as well.

The only implementation of that is in procfs.c, and it should ideally be
re-written as a gdbarch method (see comment in write_gcore_file_1), but
in the mean time I guess it doesn't hurt to throw some unique pointer at
it.

I tested that it builds on Solaris 11 (gcc compile farm machine gcc211),
but I am not able to test it, because I can't get GDB to start a
process (I'll look at that separately).

gdb/ChangeLog:

        * target.h (struct target_ops) <make_corefile_notes>:
	Change return type to unique pointer.
        * target.c (dummy_make_corefile_notes): Likewise.
        * exec.c (struct exec_target) <make_corefile_notes>:
	Likewise.
        (exec_target::make_corefile_notes): Likewise.
        * procfs.c (class procfs_target) <make_corefile_notes>:
	Likewise.
        (procfs_do_thread_registers): Adjust to unique pointer.
        (struct procfs_corefile_thread_data): Add constructor.
	<note_data>: Change type to unique pointer.
        (procfs_corefile_thread_callback): Adjust to unique pointer.
        (procfs_target::make_corefile_notes): Change return type to
	unique pointer.
        * target-delegates.c: Re-generate.
        * gcore.c (write_gcore_file_1): Adjust.
        * target-debug.h (target_debug_print_gdb_unique_xmalloc_ptr_char):
	New.

Change-Id: I768fb17ac0f7adc67d2fe95e952c784fe0ac37ab
---
 gdb/exec.c             |  4 +-
 gdb/gcore.c            |  2 +-
 gdb/procfs.c           | 88 +++++++++++++++++++++---------------------
 gdb/target-debug.h     |  2 +
 gdb/target-delegates.c | 14 +++----
 gdb/target.c           |  6 +--
 gdb/target.h           |  2 +-
 7 files changed, 61 insertions(+), 57 deletions(-)

diff --git a/gdb/exec.c b/gdb/exec.c
index dd322129139..f95fdce67f8 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -79,7 +79,7 @@ struct exec_target final : public target_ops
   void files_info () override;
 
   bool has_memory () override;
-  char *make_corefile_notes (bfd *, int *) override;
+  gdb::unique_xmalloc_ptr<char> make_corefile_notes (bfd *, int *) override;
   int find_memory_regions (find_memory_region_ftype func, void *data) override;
 };
 
@@ -1089,7 +1089,7 @@ exec_target::has_memory ()
   return !current_target_sections->empty ();
 }
 
-char *
+gdb::unique_xmalloc_ptr<char>
 exec_target::make_corefile_notes (bfd *obfd, int *note_size)
 {
   error (_("Can't create a corefile"));
diff --git a/gdb/gcore.c b/gdb/gcore.c
index 4a305ba95a8..15721f8244d 100644
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -78,7 +78,7 @@ write_gcore_file_1 (bfd *obfd)
      generation should be converted to gdbarch_make_corefile_notes; at that
      point, the target vector method can be removed.  */
   if (!gdbarch_make_corefile_notes_p (target_gdbarch ()))
-    note_data.reset (target_make_corefile_notes (obfd, &note_size));
+    note_data = target_make_corefile_notes (obfd, &note_size);
   else
     note_data = gdbarch_make_corefile_notes (target_gdbarch (), obfd,
 					     &note_size);
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 31f33fe9a2b..0b66a21c719 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -136,7 +136,7 @@ class procfs_target final : public inf_child_target
   int find_memory_regions (find_memory_region_ftype func, void *data)
     override;
 
-  char *make_corefile_notes (bfd *, int *) override;
+  gdb::unique_xmalloc_ptr<char> make_corefile_notes (bfd *, int *) override;
 
   bool info_proc (const char *, enum info_proc_what) override;
 
@@ -3495,10 +3495,10 @@ procfs_first_available (void)
 
 /* ===================  GCORE .NOTE "MODULE" =================== */
 
-static char *
+static void
 procfs_do_thread_registers (bfd *obfd, ptid_t ptid,
-			    char *note_data, int *note_size,
-			    enum gdb_signal stop_signal)
+			    gdb::unique_xmalloc_ptr<char> &note_data,
+			    int *note_size, enum gdb_signal stop_signal)
 {
   struct regcache *regcache = get_thread_regcache (&the_procfs_target, ptid);
   gdb_gregset_t gregs;
@@ -3515,25 +3515,31 @@ procfs_do_thread_registers (bfd *obfd, ptid_t ptid,
   target_fetch_registers (regcache, -1);
 
   fill_gregset (regcache, &gregs, -1);
-  note_data = (char *) elfcore_write_lwpstatus (obfd,
-						note_data,
-						note_size,
-						merged_pid,
-						stop_signal,
-						&gregs);
+  note_data.reset (elfcore_write_lwpstatus (obfd,
+					    note_data.release (),
+					    note_size,
+					    merged_pid,
+					    stop_signal,
+					    &gregs));
   fill_fpregset (regcache, &fpregs, -1);
-  note_data = (char *) elfcore_write_prfpreg (obfd,
-					      note_data,
-					      note_size,
-					      &fpregs,
-					      sizeof (fpregs));
-
-  return note_data;
+  note_data.reset (elfcore_write_prfpreg (obfd,
+					  note_data.release (),
+					  note_size,
+					  &fpregs,
+					  sizeof (fpregs)));
 }
 
-struct procfs_corefile_thread_data {
+struct procfs_corefile_thread_data
+{
+  procfs_corefile_thread_data (bfd *obfd,
+			       gdb::unique_xmalloc_ptr<char> &note_data,
+			       int *note_size, gdb_signal stop_signal)
+    : obfd (obfd), note_data (note_data), note_size (note_size),
+      stop_signal (stop_signal)
+  {}
+
   bfd *obfd;
-  char *note_data;
+  gdb::unique_xmalloc_ptr<char> &note_data;
   int *note_size;
   enum gdb_signal stop_signal;
 };
@@ -3548,10 +3554,10 @@ procfs_corefile_thread_callback (procinfo *pi, procinfo *thread, void *data)
     {
       ptid_t ptid = ptid_t (pi->pid, thread->tid, 0);
 
-      args->note_data = procfs_do_thread_registers (args->obfd, ptid,
-						    args->note_data,
-						    args->note_size,
-						    args->stop_signal);
+      procfs_do_thread_registers (args->obfd, ptid,
+				  args->note_data,
+				  args->note_size,
+				  args->stop_signal);
     }
   return 0;
 }
@@ -3578,16 +3584,15 @@ find_stop_signal (void)
     return GDB_SIGNAL_0;
 }
 
-char *
+gdb::unique_xmalloc_ptr<char>
 procfs_target::make_corefile_notes (bfd *obfd, int *note_size)
 {
   gdb_gregset_t gregs;
   char fname[16] = {'\0'};
   char psargs[80] = {'\0'};
   procinfo *pi = find_procinfo_or_die (inferior_ptid.pid (), 0);
-  char *note_data = NULL;
+  gdb::unique_xmalloc_ptr<char> note_data;
   const char *inf_args;
-  struct procfs_corefile_thread_data thread_args;
   enum gdb_signal stop_signal;
 
   if (get_exec_file (0))
@@ -3609,33 +3614,30 @@ procfs_target::make_corefile_notes (bfd *obfd, int *note_size)
 	}
     }
 
-  note_data = (char *) elfcore_write_prpsinfo (obfd,
-					       note_data,
-					       note_size,
-					       fname,
-					       psargs);
+  note_data.reset (elfcore_write_prpsinfo (obfd,
+					   note_data.release (),
+					   note_size,
+					   fname,
+					   psargs));
 
   stop_signal = find_stop_signal ();
 
   fill_gregset (get_current_regcache (), &gregs, -1);
-  note_data = elfcore_write_pstatus (obfd, note_data, note_size,
-				     inferior_ptid.pid (),
-				     stop_signal, &gregs);
-
-  thread_args.obfd = obfd;
-  thread_args.note_data = note_data;
-  thread_args.note_size = note_size;
-  thread_args.stop_signal = stop_signal;
+  note_data.reset (elfcore_write_pstatus (obfd, note_data.release (), note_size,
+					  inferior_ptid.pid (),
+					  stop_signal, &gregs));
+
+  procfs_corefile_thread_data thread_args (obfd, note_data, note_size,
+					   stop_signal);
   proc_iterate_over_threads (pi, procfs_corefile_thread_callback,
 			     &thread_args);
-  note_data = thread_args.note_data;
 
   gdb::optional<gdb::byte_vector> auxv =
     target_read_alloc (current_top_target (), TARGET_OBJECT_AUXV, NULL);
   if (auxv && !auxv->empty ())
-    note_data = elfcore_write_note (obfd, note_data, note_size,
-				    "CORE", NT_AUXV, auxv->data (),
-				    auxv->size ());
+    note_data.reset (elfcore_write_note (obfd, note_data.release (), note_size,
+					 "CORE", NT_AUXV, auxv->data (),
+					 auxv->size ()));
 
   return note_data;
 }
diff --git a/gdb/target-debug.h b/gdb/target-debug.h
index 65a14c41787..12818da6679 100644
--- a/gdb/target-debug.h
+++ b/gdb/target-debug.h
@@ -188,6 +188,8 @@
   target_debug_do_print ((X).c_str ())
 #define target_debug_print_gdb_byte_vector(X)	\
   target_debug_do_print (host_address_to_string (X.data ()))
+#define target_debug_print_gdb_unique_xmalloc_ptr_char(X) \
+  target_debug_do_print (X.get ())
 
 static void
 target_debug_print_struct_target_waitstatus_p (struct target_waitstatus *status)
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index e9c349fb7ba..437b19b8581 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -88,7 +88,7 @@ struct dummy_target : public target_ops
   bool supports_non_stop () override;
   bool always_non_stop_p () override;
   int find_memory_regions (find_memory_region_ftype arg0, void *arg1) override;
-  char *make_corefile_notes (bfd *arg0, int *arg1) override;
+  gdb::unique_xmalloc_ptr<char> make_corefile_notes (bfd *arg0, int *arg1) override;
   gdb_byte *get_bookmark (const char *arg0, int arg1) override;
   void goto_bookmark (const gdb_byte *arg0, int arg1) override;
   CORE_ADDR get_thread_local_address (ptid_t arg0, CORE_ADDR arg1, CORE_ADDR arg2) override;
@@ -259,7 +259,7 @@ struct debug_target : public target_ops
   bool supports_non_stop () override;
   bool always_non_stop_p () override;
   int find_memory_regions (find_memory_region_ftype arg0, void *arg1) override;
-  char *make_corefile_notes (bfd *arg0, int *arg1) override;
+  gdb::unique_xmalloc_ptr<char> make_corefile_notes (bfd *arg0, int *arg1) override;
   gdb_byte *get_bookmark (const char *arg0, int arg1) override;
   void goto_bookmark (const gdb_byte *arg0, int arg1) override;
   CORE_ADDR get_thread_local_address (ptid_t arg0, CORE_ADDR arg1, CORE_ADDR arg2) override;
@@ -2292,22 +2292,22 @@ debug_target::find_memory_regions (find_memory_region_ftype arg0, void *arg1)
   return result;
 }
 
-char *
+gdb::unique_xmalloc_ptr<char>
 target_ops::make_corefile_notes (bfd *arg0, int *arg1)
 {
   return this->beneath ()->make_corefile_notes (arg0, arg1);
 }
 
-char *
+gdb::unique_xmalloc_ptr<char>
 dummy_target::make_corefile_notes (bfd *arg0, int *arg1)
 {
   return dummy_make_corefile_notes (this, arg0, arg1);
 }
 
-char *
+gdb::unique_xmalloc_ptr<char>
 debug_target::make_corefile_notes (bfd *arg0, int *arg1)
 {
-  char * result;
+  gdb::unique_xmalloc_ptr<char> result;
   fprintf_unfiltered (gdb_stdlog, "-> %s->make_corefile_notes (...)\n", this->beneath ()->shortname ());
   result = this->beneath ()->make_corefile_notes (arg0, arg1);
   fprintf_unfiltered (gdb_stdlog, "<- %s->make_corefile_notes (", this->beneath ()->shortname ());
@@ -2315,7 +2315,7 @@ debug_target::make_corefile_notes (bfd *arg0, int *arg1)
   fputs_unfiltered (", ", gdb_stdlog);
   target_debug_print_int_p (arg1);
   fputs_unfiltered (") = ", gdb_stdlog);
-  target_debug_print_char_p (result);
+  target_debug_print_gdb_unique_xmalloc_ptr_char (result);
   fputs_unfiltered ("\n", gdb_stdlog);
   return result;
 }
diff --git a/gdb/target.c b/gdb/target.c
index a111ea3c333..50034cb42ec 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -89,8 +89,8 @@ static int dummy_find_memory_regions (struct target_ops *self,
 				      find_memory_region_ftype ignore1,
 				      void *ignore2);
 
-static char *dummy_make_corefile_notes (struct target_ops *self,
-					bfd *ignore1, int *ignore2);
+static gdb::unique_xmalloc_ptr<char> dummy_make_corefile_notes
+  (struct target_ops *self, bfd *ignore1, int *ignore2);
 
 static std::string default_pid_to_str (struct target_ops *ops, ptid_t ptid);
 
@@ -3058,7 +3058,7 @@ dummy_find_memory_regions (struct target_ops *self,
 }
 
 /* Error-catcher for target_make_corefile_notes.  */
-static char *
+static gdb::unique_xmalloc_ptr<char>
 dummy_make_corefile_notes (struct target_ops *self,
 			   bfd *ignore1, int *ignore2)
 {
diff --git a/gdb/target.h b/gdb/target.h
index 7e152fa6e15..039194c239b 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -719,7 +719,7 @@ struct target_ops
     virtual int find_memory_regions (find_memory_region_ftype func, void *data)
       TARGET_DEFAULT_FUNC (dummy_find_memory_regions);
     /* make_corefile_notes support method for gcore */
-    virtual char *make_corefile_notes (bfd *, int *)
+    virtual gdb::unique_xmalloc_ptr<char> make_corefile_notes (bfd *, int *)
       TARGET_DEFAULT_FUNC (dummy_make_corefile_notes);
     /* get_bookmark support method for bookmarks */
     virtual gdb_byte *get_bookmark (const char *, int)
-- 
2.28.0


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] gdb: make target_ops::make_corefile_notes return a unique ptr
  2020-10-21 19:21 [PATCH] gdb: make target_ops::make_corefile_notes return a unique ptr Simon Marchi
@ 2020-10-22 16:51 ` Tom Tromey
  2020-10-22 17:01   ` Simon Marchi
  0 siblings, 1 reply; 3+ messages in thread
From: Tom Tromey @ 2020-10-22 16:51 UTC (permalink / raw)
  To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> Since we converted gdbarch_make_corefile_notes to returning a
Simon> gdb::unique_xmalloc_ptr, I figured it would make sense to converted
Simon> target_ops::make_corefile_notes as well.

Looks good to me.  Thanks.

Tom

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] gdb: make target_ops::make_corefile_notes return a unique ptr
  2020-10-22 16:51 ` Tom Tromey
@ 2020-10-22 17:01   ` Simon Marchi
  0 siblings, 0 replies; 3+ messages in thread
From: Simon Marchi @ 2020-10-22 17:01 UTC (permalink / raw)
  To: Tom Tromey, Simon Marchi via Gdb-patches; +Cc: Simon Marchi

On 2020-10-22 12:51 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> Since we converted gdbarch_make_corefile_notes to returning a
> Simon> gdb::unique_xmalloc_ptr, I figured it would make sense to converted
> Simon> target_ops::make_corefile_notes as well.
> 
> Looks good to me.  Thanks.
> 
> Tom
> 

Thanks, pushed.

Simon

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-22 17:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-21 19:21 [PATCH] gdb: make target_ops::make_corefile_notes return a unique ptr Simon Marchi
2020-10-22 16:51 ` Tom Tromey
2020-10-22 17:01   ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).