[RFA v3] Return gdb::optional<std::string> from target_fileio_readlink

Tom Tromey
2018-02-21T21:37:49Z

This is an updated version of a patch originally submitted here:

https://sourceware.org/ml/gdb-patches/2017-11/msg00556.html

This one updates the patch as requested and fixes a couple of typos in
the commit message from the second submission.

This changes to_fileio_readlink and target_fileio_readlink to return a
gdb::optional<std::sring>, and then fixes up the callers and
implementations. This allows the removal of some cleanups.

Regression tested by the buildbot.

gdb/ChangeLog
2018-02-21 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>.
---
gdb/ChangeLog | 11 +++++++++++
gdb/inf-child.c | 12 ++++--------
gdb/linux-nat.c | 10 +++-------
gdb/linux-tdep.c | 22 ++++++++--------------
gdb/remote.c | 12 +++++-------
gdb/target.c | 12 ++++++------
gdb/target.h | 13 ++++++-------
7 files changed, 43 insertions(+), 49 deletions(-)

diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 6875596f33..c7c45530b6 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 cc930c6334..1bbad7bacc 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4709,27 +4709,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 d54e56ce8d..b5bb16215d 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -764,26 +764,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 15d6c5bdbf..7992a69e2a 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11713,7 +11713,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)
@@ -11724,10 +11724,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:");

@@ -11739,16 +11738,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 db7c09ba0f..ccce0fe73e 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3059,7 +3059,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)
{
@@ -3069,22 +3069,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 83cf48575f..05575df35f 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -935,10 +935,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. */
@@ -2091,9 +2091,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
--
2.13.6