[PATCH 0/3] Remove some uses of xrealloc

[PATCH 0/3] Remove some uses of xrealloc

[Tom Tromey]

This short series removes a few uses of xrealloc and associated manual
memory management.  This simplifies the code a bit.

Regression tested on x86-64 Fedora 36.

---
Tom Tromey (3):
      Use std::string in linux-osdata.c
      Use byte_vector in remote.c:readahead_cache
      Use std::string in do_set_command

 gdb/cli/cli-setshow.c  | 22 +++++++---------------
 gdb/nat/linux-osdata.c | 51 +++++++++++++++-----------------------------------
 gdb/remote.c           | 19 ++++++++-----------
 3 files changed, 30 insertions(+), 62 deletions(-)
---
base-commit: e11a2ebb8e44b75fad8c520a170dcce6254c5164
change-id: 20230619-remove-some-realloc-e97aad8f6481

Best regards,
-- 
Tom Tromey <tom@tromey.com>

[PATCH 1/3] Use std::string in linux-osdata.c

[Tom Tromey]

I found some code in linux-osdata that manually managed a string.
Replacing this with std::string simplifies it.
---
 gdb/nat/linux-osdata.c | 51 +++++++++++++++-----------------------------------
 1 file changed, 15 insertions(+), 36 deletions(-)

diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
index d27ddc5de23..103b7ab26cd 100644
--- a/gdb/nat/linux-osdata.c
+++ b/gdb/nat/linux-osdata.c
@@ -133,55 +133,40 @@ command_from_pid (char *command, int maxlen, PID_T pid)
   command[maxlen - 1] = '\0'; /* Ensure string is null-terminated.  */
 }
 
-/* Returns the command-line of the process with the given PID. The
-   returned string needs to be freed using xfree after use.  */
+/* Returns the command-line of the process with the given PID.  */
 
-static char *
+static std::string
 commandline_from_pid (PID_T pid)
 {
   std::string pathname = string_printf ("/proc/%lld/cmdline", pid);
-  char *commandline = NULL;
+  std::string commandline;
   gdb_file_up f = gdb_fopen_cloexec (pathname, "r");
 
   if (f)
     {
-      size_t len = 0;
-
       while (!feof (f.get ()))
 	{
 	  char buf[1024];
 	  size_t read_bytes = fread (buf, 1, sizeof (buf), f.get ());
 
 	  if (read_bytes)
-	    {
-	      commandline = (char *) xrealloc (commandline, len + read_bytes + 1);
-	      memcpy (commandline + len, buf, read_bytes);
-	      len += read_bytes;
-	    }
+	    commandline.append (buf, read_bytes);
 	}
 
-      if (commandline)
+      if (!commandline.empty ())
 	{
-	  size_t i;
-
 	  /* Replace null characters with spaces.  */
-	  for (i = 0; i < len; ++i)
-	    if (commandline[i] == '\0')
-	      commandline[i] = ' ';
-
-	  commandline[len] = '\0';
+	  for (char &c : commandline)
+	    if (c == '\0')
+	      c = ' ';
 	}
       else
 	{
 	  /* Return the command in square brackets if the command-line
 	     is empty.  */
-	  commandline = (char *) xmalloc (32);
-	  commandline[0] = '[';
-	  command_from_pid (commandline + 1, 31, pid);
-
-	  len = strlen (commandline);
-	  if (len < 31)
-	    strcat (commandline, "]");
+	  char cmd[32];
+	  command_from_pid (cmd, 31, pid);
+	  commandline = std::string ("[") + cmd + "]";
 	}
     }
 
@@ -349,7 +334,6 @@ linux_xfer_osdata_processes ()
 	  PID_T pid;
 	  uid_t owner;
 	  char user[UT_NAMESIZE];
-	  char *command_line;
 	  int *cores;
 	  int task_count;
 	  std::string cores_str;
@@ -360,7 +344,7 @@ linux_xfer_osdata_processes ()
 	    continue;
 
 	  sscanf (dp->d_name, "%lld", &pid);
-	  command_line = commandline_from_pid (pid);
+	  std::string command_line = commandline_from_pid (pid);
 
 	  if (get_process_owner (&owner, pid) == 0)
 	    user_from_uid (user, sizeof (user), owner);
@@ -393,10 +377,8 @@ linux_xfer_osdata_processes ()
 	     "</item>",
 	     pid,
 	     user,
-	     command_line ? command_line : "",
+	     command_line.c_str (),
 	     cores_str.c_str());
-
-	  xfree (command_line);
 	}
 
       closedir (dirp);
@@ -487,10 +469,9 @@ linux_xfer_osdata_processgroups ()
 	  PID_T pid = entry.pid;
 	  PID_T pgid = entry.pgid;
 	  char leader_command[32];
-	  char *command_line;
 
 	  command_from_pid (leader_command, sizeof (leader_command), pgid);
-	  command_line = commandline_from_pid (pid);
+	  std::string command_line = commandline_from_pid (pid);
 
 	  string_xml_appendf
 	    (buffer,
@@ -503,9 +484,7 @@ linux_xfer_osdata_processgroups ()
 	     pgid,
 	     leader_command,
 	     pid,
-	     command_line ? command_line : "");
-
-	  xfree (command_line);
+	     command_line.c_str ());
 	}
     }
 

-- 
2.39.2

[PATCH 2/3] Use byte_vector in remote.c:readahead_cache

[Tom Tromey]

This patch changes the remote.c readahead_cache to use
gdb::byte_vector.  This simplifies the code by removing manual memory
management.
---
 gdb/remote.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 02ff3a12bdb..513214c85a8 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -354,10 +354,7 @@ struct readahead_cache
   ULONGEST offset = 0;
 
   /* The buffer holding the cache contents.  */
-  gdb_byte *buf = nullptr;
-  /* The buffer's size.  We try to read as much as fits into a packet
-     at a time.  */
-  size_t bufsize = 0;
+  gdb::byte_vector buf;
 
   /* Cache hit and miss counters.  */
   ULONGEST hit_count = 0;
@@ -12605,14 +12602,14 @@ readahead_cache::pread (int fd, gdb_byte *read_buf, size_t len,
 {
   if (this->fd == fd
       && this->offset <= offset
-      && offset < this->offset + this->bufsize)
+      && offset < this->offset + this->buf.size ())
     {
-      ULONGEST max = this->offset + this->bufsize;
+      ULONGEST max = this->offset + this->buf.size ();
 
       if (offset + len > max)
 	len = max - offset;
 
-      memcpy (read_buf, this->buf + offset - this->offset, len);
+      memcpy (read_buf, &this->buf[offset - this->offset], len);
       return len;
     }
 
@@ -12646,10 +12643,10 @@ remote_target::remote_hostio_pread (int fd, gdb_byte *read_buf, int len,
 
   cache->fd = fd;
   cache->offset = offset;
-  cache->bufsize = get_remote_packet_size ();
-  cache->buf = (gdb_byte *) xrealloc (cache->buf, cache->bufsize);
+  cache->buf.resize (get_remote_packet_size ());
 
-  ret = remote_hostio_pread_vFile (cache->fd, cache->buf, cache->bufsize,
+  ret = remote_hostio_pread_vFile (cache->fd, &cache->buf[0],
+				   cache->buf.size (),
 				   cache->offset, remote_errno);
   if (ret <= 0)
     {
@@ -12657,7 +12654,7 @@ remote_target::remote_hostio_pread (int fd, gdb_byte *read_buf, int len,
       return ret;
     }
 
-  cache->bufsize = ret;
+  cache->buf.resize (ret);
   return cache->pread (fd, read_buf, len, offset);
 }
 

-- 
2.39.2

[PATCH 3/3] Use std::string in do_set_command

[Tom Tromey]

do_set_command manually updates a string, only to copy it to a
std::string and free the working copy.  This patch changes this code
to use std::string for everything, simplifying the code and removing a
copy.
---
 gdb/cli/cli-setshow.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index ecb739b94e9..c7bbac1666d 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -340,14 +340,12 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
     {
     case var_string:
       {
-	char *newobj;
+	std::string newobj;
 	const char *p;
-	char *q;
 	int ch;
 
-	newobj = (char *) xmalloc (strlen (arg) + 2);
+	newobj.reserve (strlen (arg));
 	p = arg;
-	q = newobj;
 	while ((ch = *p++) != '\000')
 	  {
 	    if (ch == '\\')
@@ -365,20 +363,14 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
 		if (ch == 0)
 		  break;	/* C loses */
 		else if (ch > 0)
-		  *q++ = ch;
+		  newobj.push_back (ch);
 	      }
 	    else
-	      *q++ = ch;
+	      newobj.push_back (ch);
 	  }
-#if 0
-	if (*(p - 1) != '\\')
-	  *q++ = ' ';
-#endif
-	*q++ = '\0';
-	newobj = (char *) xrealloc (newobj, q - newobj);
-
-	option_changed = c->var->set<std::string> (std::string (newobj));
-	xfree (newobj);
+	newobj.shrink_to_fit ();
+
+	option_changed = c->var->set<std::string> (std::move (newobj));
       }
       break;
     case var_string_noescape:

-- 
2.39.2

Re: [PATCH 0/3] Remove some uses of xrealloc

[John Baldwin]

On 6/19/23 10:51 AM, Tom Tromey wrote:
> This short series removes a few uses of xrealloc and associated manual
> memory management.  This simplifies the code a bit.
> 
> Regression tested on x86-64 Fedora 36.

All LGTM.

Reviewed-by: John Baldwin <jhb@FreeBSD.org>

-- 
John Baldwin