[RFA] Return unique_xmalloc_ptr from target_fileio_readlink

Message ID 877eudvt2b.fsf@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Nov. 26, 2017, 5:38 p.m. UTC
  >>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> What about gdb::optional<std::string> or std::unique_ptr<std::string>?

Sure, how about this?

Tom

commit 61fe743b565bc725972c22296b79cdb690ccf5fe
Author: Tom Tromey <tom@tromey.com>
Date:   Wed Nov 22 23:37:38 2017 -0700

    Return gdb::optional<std::string> from target_fileio_readlink
    
    This changes to_fileio_readlink and target_fileio_readlink to return a
    gdb::optional<ssd::sring>, and then fixes up the callers and
    implementations.  This allows the removal of some cleanups.
    
    Regression tested by the buildbot.
    
    ChangeLog
    2017-11-23  Tom Tromey  <tom@tromey.com>
    
            * linux-tdep.c (linux_info_proc): Update.
            * target.h (struct target_ops) <to_fileio_readlink>: Return
            optional<string>.
            (target_fileio_readlink): Return optional<string>.
            * remote.c (remote_hostio_readlink): Return optional<string>.
            * inf-child.c (inf_child_fileio_readlink): Return
            optional<string>.
            * target.c (target_fileio_readlink): Return optional<string>.
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 613a5ae6bb..66abf35a8b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@ 
+2017-11-23  Tom Tromey  <tom@tromey.com>
+
+	* linux-tdep.c (linux_info_proc): Update.
+	* target.h (struct target_ops) <to_fileio_readlink>: Return
+	optional<string>.
+	(target_fileio_readlink): Return optional<string>.
+	* remote.c (remote_hostio_readlink): Return optional<string>.
+	* inf-child.c (inf_child_fileio_readlink): Return
+	optional<string>.
+	* target.c (target_fileio_readlink): Return optional<string>.
+
 2017-11-25  Sergio Durigan Junior  <sergiodj@redhat.com>
 
 	PR gdb/22491
diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index a712613f08..63393f2253 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -333,7 +333,7 @@  inf_child_fileio_unlink (struct target_ops *self,
 
 /* Implementation of to_fileio_readlink.  */
 
-static char *
+static gdb::optional<std::string>
 inf_child_fileio_readlink (struct target_ops *self,
 			   struct inferior *inf, const char *filename,
 			   int *target_errno)
@@ -343,22 +343,18 @@  inf_child_fileio_readlink (struct target_ops *self,
 #if defined (PATH_MAX)
   char buf[PATH_MAX];
   int len;
-  char *ret;
 
   len = readlink (filename, buf, sizeof buf);
   if (len < 0)
     {
       *target_errno = host_to_fileio_error (errno);
-      return NULL;
+      return {};
     }
 
-  ret = (char *) xmalloc (len + 1);
-  memcpy (ret, buf, len);
-  ret[len] = '\0';
-  return ret;
+  return std::string (buf, len);
 #else
   *target_errno = FILEIO_ENOSYS;
-  return NULL;
+  return {};
 #endif
 }
 
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index a6395f4b5e..96a48af5dc 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4754,27 +4754,23 @@  linux_nat_fileio_open (struct target_ops *self,
 
 /* Implementation of to_fileio_readlink.  */
 
-static char *
+static gdb::optional<std::string>
 linux_nat_fileio_readlink (struct target_ops *self,
 			   struct inferior *inf, const char *filename,
 			   int *target_errno)
 {
   char buf[PATH_MAX];
   int len;
-  char *ret;
 
   len = linux_mntns_readlink (linux_nat_fileio_pid_of (inf),
 			      filename, buf, sizeof (buf));
   if (len < 0)
     {
       *target_errno = host_to_fileio_error (errno);
-      return NULL;
+      return {};
     }
 
-  ret = (char *) xmalloc (len + 1);
-  memcpy (ret, buf, len);
-  ret[len] = '\0';
-  return ret;
+  return std::string (buf, len);
 }
 
 /* Implementation of to_fileio_unlink.  */
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 24237b8d39..9a75c5a9b8 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -759,26 +759,20 @@  linux_info_proc (struct gdbarch *gdbarch, const char *args,
   if (cwd_f)
     {
       xsnprintf (filename, sizeof filename, "/proc/%ld/cwd", pid);
-      data = target_fileio_readlink (NULL, filename, &target_errno);
-      if (data)
-	{
-	  struct cleanup *cleanup = make_cleanup (xfree, data);
-          printf_filtered ("cwd = '%s'\n", data);
-	  do_cleanups (cleanup);
-	}
+      gdb::optional<std::string> contents
+	= target_fileio_readlink (NULL, filename, &target_errno);
+      if (contents.has_value ())
+	printf_filtered ("cwd = '%s'\n", contents->c_str ());
       else
 	warning (_("unable to read link '%s'"), filename);
     }
   if (exe_f)
     {
       xsnprintf (filename, sizeof filename, "/proc/%ld/exe", pid);
-      data = target_fileio_readlink (NULL, filename, &target_errno);
-      if (data)
-	{
-	  struct cleanup *cleanup = make_cleanup (xfree, data);
-          printf_filtered ("exe = '%s'\n", data);
-	  do_cleanups (cleanup);
-	}
+      gdb::optional<std::string> contents
+	= target_fileio_readlink (NULL, filename, &target_errno);
+      if (contents.has_value ())
+	printf_filtered ("exe = '%s'\n", contents->c_str ());
       else
 	warning (_("unable to read link '%s'"), filename);
     }
diff --git a/gdb/remote.c b/gdb/remote.c
index 0a62ae00e5..624e0cc390 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11711,7 +11711,7 @@  remote_hostio_unlink (struct target_ops *self,
 
 /* Implementation of to_fileio_readlink.  */
 
-static char *
+static gdb::optional<std::string>
 remote_hostio_readlink (struct target_ops *self,
 			struct inferior *inf, const char *filename,
 			int *remote_errno)
@@ -11722,10 +11722,9 @@  remote_hostio_readlink (struct target_ops *self,
   int left = get_remote_packet_size ();
   int len, attachment_len;
   int read_len;
-  char *ret;
 
   if (remote_hostio_set_filesystem (inf, remote_errno) != 0)
-    return NULL;
+    return {};
 
   remote_buffer_add_string (&p, &left, "vFile:readlink:");
 
@@ -11737,16 +11736,15 @@  remote_hostio_readlink (struct target_ops *self,
 				    &attachment_len);
 
   if (len < 0)
-    return NULL;
+    return {};
 
-  ret = (char *) xmalloc (len + 1);
+  std::string ret (len + 1, '\0');
 
   read_len = remote_unescape_input ((gdb_byte *) attachment, attachment_len,
-				    (gdb_byte *) ret, len);
+				    (gdb_byte *) &ret[0], len);
   if (read_len != len)
     error (_("Readlink returned %d, but %d bytes."), len, read_len);
 
-  ret[len] = '\0';
   return ret;
 }
 
diff --git a/gdb/target.c b/gdb/target.c
index 3bfc8b5aef..4f7390029d 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2963,7 +2963,7 @@  target_fileio_unlink (struct inferior *inf, const char *filename,
 
 /* See target.h.  */
 
-char *
+gdb::optional<std::string>
 target_fileio_readlink (struct inferior *inf, const char *filename,
 			int *target_errno)
 {
@@ -2973,22 +2973,22 @@  target_fileio_readlink (struct inferior *inf, const char *filename,
     {
       if (t->to_fileio_readlink != NULL)
 	{
-	  char *ret = t->to_fileio_readlink (t, inf, filename,
-					     target_errno);
+	  gdb::optional<std::string> ret
+	    = t->to_fileio_readlink (t, inf, filename, target_errno);
 
 	  if (targetdebug)
 	    fprintf_unfiltered (gdb_stdlog,
 				"target_fileio_readlink (%d,%s)"
 				" = %s (%d)\n",
 				inf == NULL ? 0 : inf->num,
-				filename, ret? ret : "(nil)",
-				ret? 0 : *target_errno);
+				filename, ret ? ret->c_str () : "(nil)",
+				ret ? 0 : *target_errno);
 	  return ret;
 	}
     }
 
   *target_errno = FILEIO_ENOSYS;
-  return NULL;
+  return {};
 }
 
 static void
diff --git a/gdb/target.h b/gdb/target.h
index 0d1e7bd6e8..24440770cc 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -946,10 +946,10 @@  struct target_ops
        seen by the debugger (GDB or, for remote targets, the remote
        stub).  Return a null-terminated string allocated via xmalloc,
        or NULL if an error occurs (and set *TARGET_ERRNO).  */
-    char *(*to_fileio_readlink) (struct target_ops *,
-				 struct inferior *inf,
-				 const char *filename,
-				 int *target_errno);
+    gdb::optional<std::string> (*to_fileio_readlink) (struct target_ops *,
+						      struct inferior *inf,
+						      const char *filename,
+						      int *target_errno);
 
 
     /* Implement the "info proc" command.  */
@@ -2109,9 +2109,8 @@  extern int target_fileio_unlink (struct inferior *inf,
    by the debugger (GDB or, for remote targets, the remote stub).
    Return a null-terminated string allocated via xmalloc, or NULL if
    an error occurs (and set *TARGET_ERRNO).  */
-extern char *target_fileio_readlink (struct inferior *inf,
-				     const char *filename,
-				     int *target_errno);
+extern gdb::optional<std::string> target_fileio_readlink
+    (struct inferior *inf, const char *filename, int *target_errno);
 
 /* Read target file FILENAME, in the filesystem as seen by INF.  If
    INF is NULL, use the filesystem seen by the debugger (GDB or, for