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

[Kinsbursky Stanislav]

29.02.2012 17:30, 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
>

Yes, it's guaranteed.
Moreover, It can't be less that SOCK_MIN_SNDBUF == 2048.
I don't think, that we need a BUG_ON() here.

>> +
>> +	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

Ok, I'll do so.

-- 
Best regards,
Stanislav Kinsbursky