Re: [PATCH] Introduce new shared function remote_fileio_to_fio_error

[Pedro Alves <palves@redhat.com>]

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