Re: [PATCH 12/12] linux-user: Extract target errno related functions to 'target_errno.h'

[Richard Henderson <richard.henderson@linaro.org>]

On 7/4/21 11:37 AM, Philippe Mathieu-Daudé wrote:
> Extract target errno related functions to a new 'target_errno.h'
> header, so we can do the host <-> target errno conversion out of
> the big syscall.c (which is already 13k LoC).
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   linux-user/target_errno.h |  32 +++++++
>   linux-user/syscall.c      | 162 +--------------------------------
>   linux-user/target_errno.c | 183 ++++++++++++++++++++++++++++++++++++++
>   linux-user/meson.build    |   1 +
>   4 files changed, 217 insertions(+), 161 deletions(-)
>   create mode 100644 linux-user/target_errno.h
>   create mode 100644 linux-user/target_errno.c

I guess this is just data movement, so it's ok.

But...

> +/*
> + * target_to_host_errno_table[] is initialized from
> + * host_to_target_errno_table[] in target_to_host_errno_table_init().
> + */
> +static uint16_t target_to_host_errno_table[ERRNO_TABLE_SIZE] = {
> +};
> +
> +/*
> + * This list is the union of errno values overridden in asm-<arch>/errno.h
> + * minus the errnos that are not actually generic to all archs.
> + */
> +static uint16_t host_to_target_errno_table[ERRNO_TABLE_SIZE] = {
> +    [EAGAIN]            = TARGET_EAGAIN,
> +    [EIDRM]             = TARGET_EIDRM,
> +    [ECHRNG]            = TARGET_ECHRNG,

... there's enough pattern here to make it easy to initialize both of these at 
compile-time.  We just need to move the list out to a .c.inc file.

--%<

E(EAGAIN)
E(EIDRM)
...
#ifdef EFOO
E(EFOO)
#endif

--%<

static const uint16_t target_to_host_errno_table[] = {
#define E(X)  [TARGET_##X] = X,
#include "errnos.c.inc"
#undef E
};

static const uint16_t host_to_target_errno_table[] = {
#define E(X)  [X] = TARGET_##X,
#include "errnos.c.inc"
#undef E
};

> +int host_to_target_errno(int err)
> +{
> +    if (err >= 0 && err < ERRNO_TABLE_SIZE &&
> +        host_to_target_errno_table[err]) {
> +        return host_to_target_errno_table[err];
> +    }
> +    return err;
> +}

Here and

> +int target_to_host_errno(int err)
> +{
> +    if (err >= 0 && err < ERRNO_TABLE_SIZE &&
> +        target_to_host_errno_table[err]) {
> +        return target_to_host_errno_table[err];
> +    }
> +    return err;
> +}

here, we might as well use ARRAY_SIZE(foo) instead of ERRNO_TABLE_SIZE.

Or even convert directly to switches, with no array, and let the compiler decide what it 
thinks is best.  Which might turn out to compile away to the identity function when host 
and guest are both asm-generic.


r~