Introduce new shared function remote_fileio_to_fio_error
Commit Message
Hi all,
This commit introduces a new shared function to replace three
identical functions in various places in the codebase.
Ok to commit?
Cheers,
Gary
---
gdb/ChangeLog:
* common/common-remote-fileio.h (remote_fileio_to_fio_error):
New declaration.
* common/common-remote-fileio.c (remote_fileio_to_fio_error):
New function, factored out the named functions below.
* inf-child.c (gdb/fileio.h): Remove include.
(common-remote-fileio.h): New include.
(inf_child_errno_to_fileio_error): Remove function. Update
all callers to use remote_fileio_to_fio_error.
* remote-fileio.c (remote_fileio_errno_to_target): Likewise.
gdb/gdbserver/ChangeLog:
* hostio-errno.c (errno_to_fileio_error): Remove function.
Update caller to use remote_fileio_to_fio_error.
---
gdb/ChangeLog | 12 +++++++
gdb/common/common-remote-fileio.c | 53 +++++++++++++++++++++++++++++
gdb/common/common-remote-fileio.h | 5 +++
gdb/gdbserver/ChangeLog | 5 +++
gdb/gdbserver/hostio-errno.c | 56 +-----------------------------
gdb/inf-child.c | 67 ++++--------------------------------
gdb/remote-fileio.c | 53 +----------------------------
7 files changed, 86 insertions(+), 165 deletions(-)
Comments
On 04/09/2015 10:51 AM, Gary Benson wrote:
> Hi all,
>
> This commit introduces a new shared function to replace three
> identical functions in various places in the codebase.
>
> Ok to commit?
>
This is OK, but I think the "remote" in the name is really wrong.
This is converting a _host_ errno to a fileio error. I think the
function name should reflect that, and not mention remote at all.
This is a preexisting issue with remote_fileio_to_fio_stat as well
(should be called something like host_stat_to_fio_stat / stat_to_fio_stat /
stat_to_fileio_stat / fileio_host_stat_to_fio_stat), so I'm not
pushing back on this new case. We should probably rename
the "common-remote-fileio.*" files too.
> gdb/gdbserver/ChangeLog:
>
> * hostio-errno.c (errno_to_fileio_error): Remove function.
E.g., the old name here was clear.
> Update caller to use remote_fileio_to_fio_error.
> +int
> +remote_fileio_to_fio_error (int error)
> +{
> + switch (error)
> + {
> + case EPERM:
> + return FILEIO_EPERM;
... otherwise this is truly confusing. Reading the function
name as "X to Y", makes one go "hmm, what has 'remote fileio' got
to do with about EPERM?' ?
> /* Convert a host-format mode_t into a bitmask of File-I/O flags. */
>
> static LONGEST
> diff --git a/gdb/inf-child.c b/gdb/inf-child.c
> index b7161ab..5e5763b 100644
> --- a/gdb/inf-child.c
> +++ b/gdb/inf-child.c
> @@ -311,7 +260,7 @@ inf_child_fileio_open (struct target_ops *self,
> the standard values. */
> fd = gdb_open_cloexec (filename, nat_flags, mode);
> if (fd == -1)
> - *target_errno = inf_child_errno_to_fileio_error (errno);
> + *target_errno = remote_fileio_to_fio_error (errno);
>
... and inf-child calling a "remote_"-named function is truly bizarre.
Thanks,
Pedro Alves
Pedro Alves wrote:
> On 04/09/2015 10:51 AM, Gary Benson wrote:
> > This commit introduces a new shared function to replace three
> > identical functions in various places in the codebase.
> >
> > Ok to commit?
>
> This is OK, but I think the "remote" in the name is really wrong.
> This is converting a _host_ errno to a fileio error. I think the
> function name should reflect that, and not mention remote at all.
Yeah, I kind of thought this when I was writing the patch, but I was
also thinking about other things :)
I'm happy to make another patch to rename common-remote-fileio.*
to common-fileio.* and rename the functions accordingly.
Should I push this one first and then make a second, or would you
like the rename first and an updated version of this patch second?
I'd prefer the former (less work for me!) but I can do the latter
if you want it that way.
Cheers,
Gary
On 04/09/2015 11:50 AM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 04/09/2015 10:51 AM, Gary Benson wrote:
>>> This commit introduces a new shared function to replace three
>>> identical functions in various places in the codebase.
>>>
>>> Ok to commit?
>>
>> This is OK, but I think the "remote" in the name is really wrong.
>> This is converting a _host_ errno to a fileio error. I think the
>> function name should reflect that, and not mention remote at all.
>
> Yeah, I kind of thought this when I was writing the patch, but I was
> also thinking about other things :)
>
> I'm happy to make another patch to rename common-remote-fileio.*
> to common-fileio.* and rename the functions accordingly.
I'd prefer even dropping the "common" from the file name.
host-fileio.* for example would indicate what the file actually does.
>
> Should I push this one first and then make a second, or would you
> like the rename first and an updated version of this patch second?
> I'd prefer the former (less work for me!) but I can do the latter
> if you want it that way.
Yeah, push what you have first.
Thanks,
Pedro Alves
Pedro Alves wrote:
> On 04/09/2015 11:50 AM, Gary Benson wrote:
> > Pedro Alves wrote:
> > > On 04/09/2015 10:51 AM, Gary Benson wrote:
> > > > This commit introduces a new shared function to replace three
> > > > identical functions in various places in the codebase.
> > > >
> > > > Ok to commit?
> > >
> > > This is OK, but I think the "remote" in the name is really wrong.
> > > This is converting a _host_ errno to a fileio error. I think the
> > > function name should reflect that, and not mention remote at all.
> >
> > Yeah, I kind of thought this when I was writing the patch, but I
> > was also thinking about other things :)
> >
> > I'm happy to make another patch to rename common-remote-fileio.*
> > to common-fileio.* and rename the functions accordingly.
>
> I'd prefer even dropping the "common" from the file name.
> host-fileio.* for example would indicate what the file actually does.
Or just fileio.*.
> > Should I push this one first and then make a second, or would you
> > like the rename first and an updated version of this patch second?
> > I'd prefer the former (less work for me!) but I can do the latter
> > if you want it that way.
>
> Yeah, push what you have first.
I pushed it. Will mail the renaming patch soon.
Cheers,
Gary
@@ -21,6 +21,59 @@
#include "common-remote-fileio.h"
#include <sys/stat.h>
+/* See common-remote-fileio.h. */
+
+int
+remote_fileio_to_fio_error (int error)
+{
+ switch (error)
+ {
+ case EPERM:
+ return FILEIO_EPERM;
+ case ENOENT:
+ return FILEIO_ENOENT;
+ case EINTR:
+ return FILEIO_EINTR;
+ case EIO:
+ return FILEIO_EIO;
+ case EBADF:
+ return FILEIO_EBADF;
+ case EACCES:
+ return FILEIO_EACCES;
+ case EFAULT:
+ return FILEIO_EFAULT;
+ case EBUSY:
+ return FILEIO_EBUSY;
+ case EEXIST:
+ return FILEIO_EEXIST;
+ case ENODEV:
+ return FILEIO_ENODEV;
+ case ENOTDIR:
+ return FILEIO_ENOTDIR;
+ case EISDIR:
+ return FILEIO_EISDIR;
+ case EINVAL:
+ return FILEIO_EINVAL;
+ case ENFILE:
+ return FILEIO_ENFILE;
+ case EMFILE:
+ return FILEIO_EMFILE;
+ case EFBIG:
+ return FILEIO_EFBIG;
+ case ENOSPC:
+ return FILEIO_ENOSPC;
+ case ESPIPE:
+ return FILEIO_ESPIPE;
+ case EROFS:
+ return FILEIO_EROFS;
+ case ENOSYS:
+ return FILEIO_ENOSYS;
+ case ENAMETOOLONG:
+ return FILEIO_ENAMETOOLONG;
+ }
+ return FILEIO_EUNKNOWN;
+}
+
/* Convert a host-format mode_t into a bitmask of File-I/O flags. */
static LONGEST
@@ -23,6 +23,11 @@
#include "gdb/fileio.h"
#include <sys/stat.h>
+/* Convert a errno error number to a File-I/O error number for
+ transmission over the remote protocol. */
+
+extern int remote_fileio_to_fio_error (int error);
+
/* Pack a host-format integer into a byte buffer in big-endian format
ready for transmission over the remote protocol. BYTES specifies
the size of the integer to pack in bytes. */
@@ -22,64 +22,12 @@
on top of errno. */
#include "server.h"
-#include "gdb/fileio.h"
-
-static int
-errno_to_fileio_error (int error)
-{
- switch (error)
- {
- case EPERM:
- return FILEIO_EPERM;
- case ENOENT:
- return FILEIO_ENOENT;
- case EINTR:
- return FILEIO_EINTR;
- case EIO:
- return FILEIO_EIO;
- case EBADF:
- return FILEIO_EBADF;
- case EACCES:
- return FILEIO_EACCES;
- case EFAULT:
- return FILEIO_EFAULT;
- case EBUSY:
- return FILEIO_EBUSY;
- case EEXIST:
- return FILEIO_EEXIST;
- case ENODEV:
- return FILEIO_ENODEV;
- case ENOTDIR:
- return FILEIO_ENOTDIR;
- case EISDIR:
- return FILEIO_EISDIR;
- case EINVAL:
- return FILEIO_EINVAL;
- case ENFILE:
- return FILEIO_ENFILE;
- case EMFILE:
- return FILEIO_EMFILE;
- case EFBIG:
- return FILEIO_EFBIG;
- case ENOSPC:
- return FILEIO_ENOSPC;
- case ESPIPE:
- return FILEIO_ESPIPE;
- case EROFS:
- return FILEIO_EROFS;
- case ENOSYS:
- return FILEIO_ENOSYS;
- case ENAMETOOLONG:
- return FILEIO_ENAMETOOLONG;
- }
-
- return FILEIO_EUNKNOWN;
-}
+#include "common-remote-fileio.h"
void
hostio_last_error_from_errno (char *buf)
{
int error = errno;
- int fileio_error = errno_to_fileio_error (error);
+ int fileio_error = remote_fileio_to_fio_error (error);
sprintf (buf, "F-1,%x", fileio_error);
}
@@ -30,7 +30,7 @@
#include "inferior.h"
#include <sys/stat.h>
#include "inf-child.h"
-#include "gdb/fileio.h"
+#include "common-remote-fileio.h"
#include "agent.h"
#include "gdb_wait.h"
#include "filestuff.h"
@@ -239,57 +239,6 @@ inf_child_fileio_open_flags_to_host (int fileio_open_flags, int *open_flags_p)
return 0;
}
-static int
-inf_child_errno_to_fileio_error (int errnum)
-{
- switch (errnum)
- {
- case EPERM:
- return FILEIO_EPERM;
- case ENOENT:
- return FILEIO_ENOENT;
- case EINTR:
- return FILEIO_EINTR;
- case EIO:
- return FILEIO_EIO;
- case EBADF:
- return FILEIO_EBADF;
- case EACCES:
- return FILEIO_EACCES;
- case EFAULT:
- return FILEIO_EFAULT;
- case EBUSY:
- return FILEIO_EBUSY;
- case EEXIST:
- return FILEIO_EEXIST;
- case ENODEV:
- return FILEIO_ENODEV;
- case ENOTDIR:
- return FILEIO_ENOTDIR;
- case EISDIR:
- return FILEIO_EISDIR;
- case EINVAL:
- return FILEIO_EINVAL;
- case ENFILE:
- return FILEIO_ENFILE;
- case EMFILE:
- return FILEIO_EMFILE;
- case EFBIG:
- return FILEIO_EFBIG;
- case ENOSPC:
- return FILEIO_ENOSPC;
- case ESPIPE:
- return FILEIO_ESPIPE;
- case EROFS:
- return FILEIO_EROFS;
- case ENOSYS:
- return FILEIO_ENOSYS;
- case ENAMETOOLONG:
- return FILEIO_ENAMETOOLONG;
- }
- return FILEIO_EUNKNOWN;
-}
-
/* Open FILENAME on the target, using FLAGS and MODE. Return a
target file descriptor, or -1 if an error occurs (and set
*TARGET_ERRNO). */
@@ -311,7 +260,7 @@ inf_child_fileio_open (struct target_ops *self,
the standard values. */
fd = gdb_open_cloexec (filename, nat_flags, mode);
if (fd == -1)
- *target_errno = inf_child_errno_to_fileio_error (errno);
+ *target_errno = remote_fileio_to_fio_error (errno);
return fd;
}
@@ -340,7 +289,7 @@ inf_child_fileio_pwrite (struct target_ops *self,
}
if (ret == -1)
- *target_errno = inf_child_errno_to_fileio_error (errno);
+ *target_errno = remote_fileio_to_fio_error (errno);
return ret;
}
@@ -369,7 +318,7 @@ inf_child_fileio_pread (struct target_ops *self,
}
if (ret == -1)
- *target_errno = inf_child_errno_to_fileio_error (errno);
+ *target_errno = remote_fileio_to_fio_error (errno);
return ret;
}
@@ -383,7 +332,7 @@ inf_child_fileio_fstat (struct target_ops *self, int fd,
ret = fstat (fd, sb);
if (ret == -1)
- *target_errno = inf_child_errno_to_fileio_error (errno);
+ *target_errno = remote_fileio_to_fio_error (errno);
return ret;
}
@@ -397,7 +346,7 @@ inf_child_fileio_close (struct target_ops *self, int fd, int *target_errno)
ret = close (fd);
if (ret == -1)
- *target_errno = inf_child_errno_to_fileio_error (errno);
+ *target_errno = remote_fileio_to_fio_error (errno);
return ret;
}
@@ -412,7 +361,7 @@ inf_child_fileio_unlink (struct target_ops *self,
ret = unlink (filename);
if (ret == -1)
- *target_errno = inf_child_errno_to_fileio_error (errno);
+ *target_errno = remote_fileio_to_fio_error (errno);
return ret;
}
@@ -434,7 +383,7 @@ inf_child_fileio_readlink (struct target_ops *self,
len = readlink (filename, buf, sizeof buf);
if (len < 0)
{
- *target_errno = inf_child_errno_to_fileio_error (errno);
+ *target_errno = remote_fileio_to_fio_error (errno);
return NULL;
}
@@ -194,57 +194,6 @@ remote_fileio_mode_to_host (long mode, int open_call)
}
static int
-remote_fileio_errno_to_target (int error)
-{
- switch (error)
- {
- case EPERM:
- return FILEIO_EPERM;
- case ENOENT:
- return FILEIO_ENOENT;
- case EINTR:
- return FILEIO_EINTR;
- case EIO:
- return FILEIO_EIO;
- case EBADF:
- return FILEIO_EBADF;
- case EACCES:
- return FILEIO_EACCES;
- case EFAULT:
- return FILEIO_EFAULT;
- case EBUSY:
- return FILEIO_EBUSY;
- case EEXIST:
- return FILEIO_EEXIST;
- case ENODEV:
- return FILEIO_ENODEV;
- case ENOTDIR:
- return FILEIO_ENOTDIR;
- case EISDIR:
- return FILEIO_EISDIR;
- case EINVAL:
- return FILEIO_EINVAL;
- case ENFILE:
- return FILEIO_ENFILE;
- case EMFILE:
- return FILEIO_EMFILE;
- case EFBIG:
- return FILEIO_EFBIG;
- case ENOSPC:
- return FILEIO_ENOSPC;
- case ESPIPE:
- return FILEIO_ESPIPE;
- case EROFS:
- return FILEIO_EROFS;
- case ENOSYS:
- return FILEIO_ENOSYS;
- case ENAMETOOLONG:
- return FILEIO_ENAMETOOLONG;
- }
- return FILEIO_EUNKNOWN;
-}
-
-static int
remote_fileio_seek_flag_to_host (long num, int *flag)
{
if (!flag)
@@ -459,7 +408,7 @@ static void
remote_fileio_return_errno (int retcode)
{
remote_fileio_reply (retcode, retcode < 0
- ? remote_fileio_errno_to_target (errno) : 0);
+ ? remote_fileio_to_fio_error (errno) : 0);
}
static void