From patchwork Wed Mar 7 22:44:37 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 26239 Received: (qmail 113778 invoked by alias); 7 Mar 2018 22:44:45 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 113751 invoked by uid 89); 7 Mar 2018 22:44:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: gateway21.websitewelcome.com Received: from gateway21.websitewelcome.com (HELO gateway21.websitewelcome.com) (192.185.45.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 07 Mar 2018 22:44:41 +0000 Received: from cm14.websitewelcome.com (cm14.websitewelcome.com [100.42.49.7]) by gateway21.websitewelcome.com (Postfix) with ESMTP id 78102400ED719 for ; Wed, 7 Mar 2018 16:44:40 -0600 (CST) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id thncemTsvE0sxthncemos9; Wed, 07 Mar 2018 16:44:40 -0600 Received: from 174-29-60-18.hlrn.qwest.net ([174.29.60.18]:52646 helo=bapiya) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89) (envelope-from ) id 1ethnb-001crV-0u; Wed, 07 Mar 2018 16:44:39 -0600 From: Tom Tromey To: Simon Marchi Cc: Tom Tromey , Subject: Re: [RFA v3] Return gdb::optional from target_fileio_readlink References: <20180221213741.19409-1-tom@tromey.com> Date: Wed, 07 Mar 2018 15:44:37 -0700 In-Reply-To: (Simon Marchi's message of "Wed, 7 Mar 2018 15:37:35 -0500") Message-ID: <87d10fo696.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.91 (gnu/linux) MIME-Version: 1.0 X-BWhitelist: no X-Source-L: No X-Exim-ID: 1ethnb-001crV-0u X-Source-Sender: 174-29-60-18.hlrn.qwest.net (bapiya) [174.29.60.18]:52646 X-Source-Auth: tom+tromey.com X-Email-Count: 2 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes >>>>> "Simon" == Simon Marchi writes: Simon> I think it should be just "len" and not "len + 1" here. The NULL byte is added by Simon> the std::string on top of that length. remote_unescape_input will only write len Simon> bytes, so it won't touch that NULL byte. Makes sense. Simon> Can you update the comment above this? Simon> LGTM with that fixed. Here's what I'm checking in. Tom commit e0d3522b888033febd153a4a7d3b7c5c13641f09 Author: Tom Tromey Date: Wed Nov 22 23:37:38 2017 -0700 Return gdb::optional from target_fileio_readlink This changes to_fileio_readlink and target_fileio_readlink to return a gdb::optional, and then fixes up the callers and implementations. This allows the removal of some cleanups. Regression tested by the buildbot. gdb/ChangeLog 2018-03-07 Tom Tromey * linux-tdep.c (linux_info_proc): Update. * target.h (struct target_ops) : Return optional. (target_fileio_readlink): Return optional. * remote.c (remote_hostio_readlink): Return optional. * inf-child.c (inf_child_fileio_readlink): Return optional. * target.c (target_fileio_readlink): Return optional. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 928dbab31f..49b91e1e15 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,14 @@ +2018-03-07 Tom Tromey + + * linux-tdep.c (linux_info_proc): Update. + * target.h (struct target_ops) : Return + optional. + (target_fileio_readlink): Return optional. + * remote.c (remote_hostio_readlink): Return optional. + * inf-child.c (inf_child_fileio_readlink): Return + optional. + * target.c (target_fileio_readlink): Return optional. + 2018-03-07 Andrew Burgess * regcache.c (cooked_read_test): Add riscv to the list of 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 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 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 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 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..134a97eed9 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 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, '\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 197e7ff542..2b97118217 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 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 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 de562139c7..fd6c30b983 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -933,12 +933,12 @@ struct target_ops /* Read value of symbolic link FILENAME on the target, in the filesystem as seen by INF. If INF is NULL, use the filesystem 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); + stub). Return a string, or an empty optional if an error + occurs (and set *TARGET_ERRNO). */ + gdb::optional (*to_fileio_readlink) (struct target_ops *, + struct inferior *inf, + const char *filename, + int *target_errno); /* Implement the "info proc" command. */ @@ -2095,9 +2095,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 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