On 2018-02-21 04:37 PM, Tom Tromey wrote:
> --- 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');
I think it should be just "len" and not "len + 1" here. The NULL byte is added by
the std::string on top of that length. remote_unescape_input will only write len
bytes, so it won't touch that NULL byte.
> 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);
Can you update the comment above this?
LGTM with that fixed.
Simon
@@ -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
}
@@ -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. */
@@ -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);
}
@@ -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;
}
@@ -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
@@ -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