Re: [PATCH] linux-user: Support for NETLINK socket options

[Laurent Vivier]

Le 29/10/2019 à 23:43, Josh Kunz a écrit :
> This change includes support for all AF_NETLINK socket options up to about
> kernel version 5.4 (5.4 is not formally released at the time of writing).
> Socket options that were introduced in kernel versions before the oldest
> currently stable kernel version are guarded by kernel version macros.
> 
> This change has been built under gcc 8.3, and clang 9.0, and it passes
> `make check`. The netlink options have been tested by emulating some
> non-trival software that uses NETLINK socket options, but they have
> not been exaustively verified.
> 
> Signed-off-by: Josh Kunz <address@hidden>
> ---
>  linux-user/syscall.c | 98 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
...
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 2, 0)
> +        case NETLINK_LIST_MEMBERSHIPS:
> +        {
> +            if (get_user_u32(len, optlen)) {
> +                return -TARGET_EFAULT;
> +            }
> +            if (len < 0) {
> +                return -TARGET_EINVAL;
> +            }
> +            uint32_t *results = lock_user(VERIFY_WRITE, optval_addr, len, 1);

Please put the declaration of "results" at the beginning of the block.
See CODING_STYLE file, "Declarations"

> +            if (!results) {
> +                return -TARGET_EFAULT;
> +            }
> +            lv = len;
> +            ret = get_errno(getsockopt(sockfd, level, optname, results, 
> &lv));
> +            if (ret < 0) {
> +                unlock_user(results, optval_addr, 0);
> +                return ret;
> +            }
> +            /* swap host endianess to target endianess. */
> +            for (int i = 0; i < (len / sizeof(uint32_t)); i++) {

Put the declaration of "i" at the beginning of the block.

Otherwise, it looks good.

Thanks,
Laurent