[CRIU] Re: [PATCH v2 2/9] parasite: support sockets queues

[Cyrill Gorcunov]

On Wed, Feb 29, 2012 at 04:06:40PM +0300, Kinsbursky Stanislav wrote:
...
> +static int dump_socket_queue(int img_fd, struct sk_queue_item *item, int *err)
> +{
> +	struct sk_packet_entry *pe;
> +	unsigned long size;
> +	socklen_t tmp;
> +	int ret, orig_peek_off;
> +	int sock_fd = item->fd;
> +
> +	/*
> +	 * Save original peek offset. 
> +	 */
> +	ret = sys_getsockopt(sock_fd, SOL_SOCKET, SO_PEEK_OFF, &orig_peek_off, &tmp);
> +	if (ret < 0) {
> +		sys_write_msg("getsockopt failed\n");
> +		*err = ret;
> +		return PARASITE_ERR_FAIL;
> +	}
> +	/*
> +	 * Discover max DGRAM size
> +	 */
> +	ret = sys_getsockopt(sock_fd, SOL_SOCKET, SO_SNDBUF, &ret, &tmp);
> +	if (ret < 0) {
> +		sys_write_msg("getsockopt failed\n");
> +		*err = ret;
> +		return PARASITE_ERR_FAIL;
> +	}
> +	/*
> +	 * Note: 32 bytes will be used by kernel for protocol header.
> +	 */
> +	size = ret - 32;

Is it possible that ret will be < 32? If yet, size may gen hight bit
set and since it's unsigned the huge number will be there, I guess
it's not what we expecting for.

If ret is guaranteed to be > 32 than BUG_ON(ret <= 32) I think

> +
> +	while (1) {
> +		struct iovec iov = {
> +			.iov_base = pe->data,
> +			.iov_len = size,
> +		};
> +		struct msghdr msg = {
> +			.msg_name = NULL,
> +			.msg_namelen = 0,
> +			.msg_iov = &iov,
> +			.msg_iovlen = 1,
> +			.msg_control = NULL,
> +			.msg_controllen = 0,
> +			.msg_flags = 0,
> +		};

Stas, please align multiple assignments, it's a way easier to read.

	struct iovec iov = {
		.iov_base	= pe->data,
		.iov_len	= size,
	};

	struct msghdr msg = {
		.msg_name	= NULL,
		.msg_namelen	= 0,
		.msg_iov	= &iov,
		.msg_iovlen	= 1,
		.msg_control	= NULL,
		.msg_controllen	= 0,
		.msg_flags	= 0,
	};

Actually there is no need to zeroify the members explicitly.
But if you prefer -- I'm fine with it :)

	Cyrill