[CRIU] Re: [PATCH 3/3] util-net: Add ability to send/receive arrays of file descriptors

[Pavel Emelyanov]

> +static always_inline void *builtin_memcpy(void *to, const void *from, unsigned int n)
> +{
> +	int d0, d1, d2;
> +	asm volatile("rep ; movsl		\n"
> +		     "movl %4,%%ecx		\n"
> +		     "andl $3,%%ecx		\n"
> +		     "jz 1f			\n"
> +		     "rep ; movsb		\n"
> +		     "1:"
> +		     : "=&c" (d0), "=&D" (d1), "=&S" (d2)
> +		     : "0" (n / 4), "g" (n), "1" ((long)to), "2" ((long)from)
> +		     : "memory");
> +	return to;

What for? The glibc memcpy just works.
But anyway, even if we do need it it should go in separate patch.

> +struct scm_fdset {
> +	struct msghdr		hdr;
> +	struct iovec		iov;
> +	char			msg_buf[CR_SCM_MSG_SIZE];
> +	int			__pad;
> +	union {
> +		int		nr_fds_rx;
> +		int		nr_fds_tx;
> +		int		__nr_fds;

This union-ing is obfuscating.

> +	};
> +};

> +extern int scm_fdset_update(struct scm_fdset *fdset, int nr_fds);
> +extern int *scm_fdset_first(struct scm_fdset *fdset);
> +extern int *scm_fdset_init(struct scm_fdset *fdset);
> +extern void scm_fdset_set_addr(struct scm_fdset *fdset, struct sockaddr_un *saddr, int saddr_len);
> +extern int scm_fdset_send(int sock, struct scm_fdset *fdset);
> +extern int scm_fdset_recv(int sock, struct scm_fdset *fdset);
> +extern int send_fds(int sock, struct sockaddr_un *saddr, int saddr_len, int *fds, int nr_fds);
> +extern int recv_fds(int sock, int *fds, int nr_fds);

This whole bunch of functions
a) is not used outside util-net.c
b) has wrong declared return arguments versus used by callers

> -int recv_fd(int sock)
> +void scm_fdset_set_addr(struct scm_fdset *fdset, struct sockaddr_un *saddr, int saddr_len)
>  {
> -	char ccmsg[CMSG_SPACE(sizeof(int))];
> -	struct msghdr msg = { };
> -	struct iovec iov = { };
> -	struct cmsghdr *cmsg;
> -	int *cmsg_data;
> -	char buf[1];
> -	int ret;

It's worth splitting this patch into
3.1 put all the msg, iov, cmsg, etc. onto a helper bounding structure
3.2 add support for multiple fds sending/receiving
to make the review easier.

> +int send_fds(int sock, struct sockaddr_un *saddr, int saddr_len, int *fds, int nr_fds)
> +{
> +	struct scm_fdset fdset;
> +	int *__fds_tx;
> +	int i, j, ret;
> +
> +	scm_fdset_init(&fdset);
> +	scm_fdset_set_addr(&fdset, saddr, saddr_len);

_init and _set_addr are never used separately. It worth replacing two calls with one.