Re: [Qemu-devel] [PATCH] linux-user: add string type in rtentry struct

[Peter Maydell]

On 20 December 2012 20:56, Laurent Vivier <address@hidden> wrote:
> This allows to pass the device name.

> +static void target_to_host_string (void *dst, const void *src)
> +{
> +#if HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 32
> +    if (*(uint32_t*)src == 0) {
> +        *(uint32_t*)dst = 0;
> +       return;
> +    }
> +    *(uint32_t *)dst = (uint32_t)g2h(tswap32(*(uint32_t *)src));
> +#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 32
> +    if (*(uint32_t*)src == 0) {
> +        *(uint64_t*)dst = 0;
> +       return;
> +    }
> +    *(uint64_t *)dst = (uint64_t)g2h(tswap32(*(uint32_t *)src));
> +#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 64
> +    if (*(uint64_t*)src == 0) {
> +        *(uint64_t*)dst = 0;
> +       return;
> +    }
> +    *(uint64_t *)dst = (uint64_t)g2h(tswap64(*(uint64_t *)src));
> +#elif HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 64
> +    if (*(uint64_t*)src == 0) {
> +        *(uint32_t*)dst = 0;
> +       return;
> +    }
> +    *(uint32_t *)dst = (uint32_t)g2h(tswap64(*(uint64_t *)src));
> +#endif
> +}
> +
> +static void host_to_target_string (void *dst, const void *src)
> +{
> +#if HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 32
> +    if (*(uint32_t*)src == 0) {
> +        *(uint32_t*)dst = 0;
> +       return;
> +    }
> +    *(uint32_t *)dst = tswap32(h2g(*(uint32_t *)src));
> +#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 32
> +    if (*(uint64_t*)src == 0) {
> +        *(uint32_t*)dst = 0;
> +       return;
> +    }
> +    *(uint32_t *)dst = tswap32(h2g(*(uint64_t *)src));
> +#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 64
> +    if (*(uint64_t*)src == 0) {
> +        *(uint64_t*)dst = 0;
> +       return;
> +    }
> +    *(uint64_t *)dst = tswap64(h2g(*(uint64_t *)src));
> +#elif HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 64
> +    if (*(uint32_t*)src == 0) {
> +        *(uint64_t*)dst = 0;
> +       return;
> +    }
> +    *(uint64_t *)dst = tswap64(h2g(*(uint32_t *)src));
> +#endif
> +}
> +
> +static const StructEntry struct_string_def = {
> +    .convert = { host_to_target_string, target_to_host_string },
> +    .size = { sizeof(target_long), sizeof(long) },
> +    .align = { __alignof__(target_long), __alignof__(long) },
> +};

This is the wrong approach, I'm afraid. Among other problems,
you don't have anywhere to catch the case of being passed a bad pointer
(should fail EFAULT). In general, the thunk_convert routines aren't designed
to handle converting pointed-to data and your patch is abusing MK_STRUCT
(which is for embedded structs, not pointed-to anything). Unless you feel
like doing the redesign of the thunk conversion code, the right way to handle
oddball ioctl parameters is to write a do_ioctl_fn for them (compare
SIOCGIFCONF, for example).

The kernel sources:
http://lxr.linux.no/#linux+v3.7.1/net/socket.c#L3175
show that it always copies a fixed 15 bytes, incidentally.

-- PMM