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: [PATCH v2 5/5] Eliminate make_cleanup_ui_file_delete / make ui_file a class hierarchy


On 2017-01-23 18:18, Pedro Alves wrote:
diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index ea64220..cc14d9d 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -1762,43 +1762,42 @@ aix_thread_extra_thread_info (struct target_ops *self,
   if (!PD_TID (thread->ptid))
     return NULL;

-  buf = mem_fileopen ();
+  string_file buf;

You did not remove the old declaration of buf.


   pdtid = thread->priv->pdtid;
   tid = thread->priv->tid;

   if (tid != PTHDB_INVALID_TID)
/* i18n: Like "thread-identifier %d, [state] running, suspended" */
-    fprintf_unfiltered (buf, _("tid %d"), (int)tid);
+    buf.printf (_("tid %d"), (int)tid);

   status = pthdb_pthread_state (pd_session, pdtid, &state);
   if (status != PTHDB_SUCCESS)
     state = PST_NOTSUP;
-  fprintf_unfiltered (buf, ", %s", state2str (state));
+  buf.printf (", %s", state2str (state));

   status = pthdb_pthread_suspendstate (pd_session, pdtid,
 				       &suspendstate);
   if (status == PTHDB_SUCCESS && suspendstate == PSS_SUSPENDED)
     /* i18n: Like "Thread-Id %d, [state] running, suspended" */
-    fprintf_unfiltered (buf, _(", suspended"));
+    buf.printf (_(", suspended"));

   status = pthdb_pthread_detachstate (pd_session, pdtid,
 				      &detachstate);
   if (status == PTHDB_SUCCESS && detachstate == PDS_DETACHED)
     /* i18n: Like "Thread-Id %d, [state] running, detached" */
-    fprintf_unfiltered (buf, _(", detached"));
+    buf.printf (_(", detached"));

   pthdb_pthread_cancelpend (pd_session, pdtid, &cancelpend);
   if (status == PTHDB_SUCCESS && cancelpend)
     /* i18n: Like "Thread-Id %d, [state] running, cancel pending" */
-    fprintf_unfiltered (buf, _(", cancel pending"));
+    buf.printf (_(", cancel pending"));

-  ui_file_write (buf, "", 1);
+  buf.write ("", 1);

   xfree (ret);			/* Free old buffer.  */

-  ret = ui_file_xstrdup (buf, NULL);
-  ui_file_delete (buf);
+  ret = xstrdup (buf.string ());

Can xstrdup take an std::string? Implicit conversions only go the other way I think.

It might be worth sending the series on the buildbot to test AIX.


   return ret;
 }
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 2bdfa57..bc170b1 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -9573,7 +9573,6 @@ extern initialize_file_ftype
_initialize_arm_tdep; /* -Wmissing-prototypes */
 void
 _initialize_arm_tdep (void)
 {
-  struct ui_file *stb;
   long length;
   const char *setname;
   const char *setdesc;
@@ -9645,13 +9644,12 @@ _initialize_arm_tdep (void)
   valid_disassembly_styles[num_disassembly_options] = NULL;

   /* Create the help text.  */
-  stb = mem_fileopen ();
-  fprintf_unfiltered (stb, "%s%s%s",
-		      _("The valid values are:\n"),
-		      regdesc,
-		      _("The default is \"std\"."));
-  helptext = ui_file_as_string (stb);
-  ui_file_delete (stb);
+  string_file stb;
+  stb.printf ("%s%s%s",
+	      _("The valid values are:\n"),
+	      regdesc,
+	      _("The default is \"std\"."));
+  helptext = std::move (stb.string ());

I think having that static helptext variable is useless here. The help text is copied in add_setshow_enum_cmd, so we don't need to keep helptext for the whole lifetime of the program. We could pass stb.c_str () directly to add_setshow_enum_cmd.

+class string_file : public ui_file
+{
+public:
+  string_file () {}
+  ~string_file () override;

-/* Returns a std::string containing the entire contents of FILE (as
-   determined by ui_file_put()).  */
-extern std::string ui_file_as_string (struct ui_file *file);
+  /* Override ui_file methods.  */

-/* Similar to ui_file_xstrdup, but return a new string allocated on
-   OBSTACK.  */
-extern char *ui_file_obsavestring (struct ui_file *file,
-				   struct obstack *obstack, long *length);
+  void write (const char *buf, long length_buf) override;

-extern long ui_file_read (struct ui_file *file, char *buf, long length_buf);
+  long read (char *buf, long length_buf) override
+  { gdb_assert_not_reached ("a string_file is not readable"); }
+
+  /* string_file-specific public API.  */
+
+  /* Accesses the std::string containing the entire output collected
+     so far.  Returns a non-const reference so that it's easy to move
+     the string contents out of the string_file.  */

I didn't understand what you meant by "to move the string contents out..." until I saw this

  helptext = std::move (stb.string ());

If it had been "to std::move the string contents out..." it might have been a bit clearer, I'm not entirely sure.

--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -187,18 +187,6 @@ make_cleanup_obstack_free (struct obstack *obstack)
   return make_cleanup (do_obstack_free, obstack);
 }

-static void
-do_ui_file_delete (void *arg)
-{
-  ui_file_delete ((struct ui_file *) arg);
-}
-
-struct cleanup *
-make_cleanup_ui_file_delete (struct ui_file *arg)
-{
-  return make_cleanup (do_ui_file_delete, arg);
-}
-

You can remove the declaration in utils.h for this.

That's all for me.

Thanks!

Simon


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