[CRIU] Re: [PATCH 1/9] protobuf: Convert sk_opts_entry to PB format

[Pavel Emelyanov]

On 07/18/2012 04:13 PM, Pavel Emelyanov wrote:
> On 07/18/2012 12:21 PM, Cyrill Gorcunov wrote:
>>
>> This patch prepares the ground for further patches.
>> sk_opts_entry get converted to PB format but not
>> yet used anywhere in code. Also a few additional
>> pb_ helpers are added.
>>
>> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
>> ---
>>  include/sockets.h      |    8 ++++++
>>  protobuf/Makefile      |    1 +
>>  protobuf/sk-opts.proto |    7 +++++
>>  sockets.c              |   65 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 81 insertions(+), 0 deletions(-)
>>  create mode 100644 protobuf/sk-opts.proto
>>
> 
>> +message sk_opts_entry {
>> +	required uint32		so_sndbuf	= 1;
>> +	required uint32		so_rcvbuf	= 2;
>> +
>> +	repeated uint64		so_snd_tmo	= 3;
>> +	repeated uint64		so_rcv_tmo	= 4;
>> +}
> 
> Here's what I thought. These times are effectively struct timeval
> date. Thus we _should_ declare these in proto file as
> 
> 	required uint64	so_snd_tmo_sec = 3;
> 	required uint64 so_snd_tmo_usec = 4;
> 	required uint64	so_rcv_tmo_sec = 5;
> 	required uint64 so_rcv_tmo_usec = 6;
> 
> Next. This:
> 
>> +int pb_dump_socket_opts(int sk, SkOptsEntry *soe)
>> +{
>> ...
>> +	size = pb_repeated_size(soe, so_snd_tmo);
>> +
>> ...
>> +	ret |= do_dump_opt(sk, SO_SNDTIMEO, soe->so_snd_tmo, size);
>> +	ret |= do_dump_opt(sk, SO_RCVTIMEO, soe->so_rcv_tmo, size);
>> ...
>> +}
> 
> is not what I asked for. You should not mix protobuf data types with C ones.
> Taking the above into account this (and the restore part which you didn't
> touch) will get properly fixed like this:
> 
> 	struct timeval snd;
> 
> 	snd.tv_sec = soe->so_snd_tmo_sec;
> 	snd.tv_usec = soe->so_snd_tmo_usec;
> 
> 	ret |= do_restore_opt(sk, SO_SNDTIMEO, &snd, sizeof(snd));
> 
> Please.

Never mind, it's easy, I did it myself. Patches 1-3 applied.