[CRIU] [PATCH] net/sysctl: remove excess type conversions for sysctl_entry.type

[Pavel Emelyanov]

On 05/18/2016 02:05 PM, Pavel Tikhomirov wrote:
> 
> 
> On 05/17/2016 02:52 PM, Pavel Emelyanov wrote:
>> On 05/16/2016 05:32 PM, Pavel Tikhomirov wrote:
>>> use SYSCTL_TYPE__CTL_32 and SYSCTL_TYPE____CTL_STR from SysctlEntry enum
>>>
>>> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
>>> ---
>>>  criu/net.c | 29 +++++++++++++++--------------
>>>  1 file changed, 15 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/criu/net.c b/criu/net.c
>>> index 59c1161..a28ad80 100644
>>> --- a/criu/net.c
>>> +++ b/criu/net.c
>>> @@ -66,11 +66,12 @@ static bool sysctl_entries_equal(SysctlEntry *a, SysctlEntry *b)
>>>  	if (a->type != b->type)
>>>  		return false;
>>>
>>> -	switch (CTL_TYPE(a->type)) {
>>> -		case CTL_32:
>>> +	switch (a->type) {
>>> +		case SYSCTL_TYPE__CTL_32:
>>
>> Should SYSCTL_TYPE__CTL_32 == CTL_32?
> 
> Here SYSCTL_TYPE__CTL_32 is enumeration constant and CTL_32 is 
> preprocessor constant and they are both equal to 6 of course. But it 
> seem to me that using enumeration constants when work with enumeration 
> variable is a bit clearer.

So should they match or not? If yes, add BUILD_BUG_ONS-s

>>
>>>  			return a->has_iarg && b->has_iarg && a->iarg == b->iarg;
>>> -		case __CTL_STR:
>>> +		case SYSCTL_TYPE____CTL_STR:
>>
>> Any reason we need 4 underscores here?
> 
> We have CTL_STR(len) - string with length and __CTL_STR for string 
> without length. So I named the enumeration const in SysctlType similarly 
> __CTL_STR as it do not save lenght. And protobuf adds SYSCTL_TYPE__ 
> prefix to all enum consts. These way I got SYSCTL_TYPE____CTL_STR with 4 
> underscores, no other reason.

Please, drop extra underscores from pb enum.

>>
>>>  			return a->sarg && b->sarg && !strcmp(a->sarg, b->sarg);
>>> +		default:;
>>>  	}
>>>
>>>  	return false;
>>> @@ -191,8 +192,8 @@ static int net_conf_op(char *tgt, SysctlEntry **conf, int n, int op, char *proto
>>>
>>>  		snprintf(path[i], MAX_CONF_OPT_PATH, CONF_OPT_PATH, proto, tgt, devconfs[i]);
>>>  		req[ri].name = path[i];
>>> -		switch (CTL_TYPE(conf[i]->type)) {
>>> -			case CTL_32:
>>> +		switch (conf[i]->type) {
>>> +			case SYSCTL_TYPE__CTL_32:
>>>  				req[ri].type = CTL_32;
>>>
>>>  				/* skip non-existing sysctl */
>>> @@ -201,7 +202,7 @@ static int net_conf_op(char *tgt, SysctlEntry **conf, int n, int op, char *proto
>>>
>>>  				req[ri].arg = &conf[i]->iarg;
>>>  				break;
>>> -			case __CTL_STR:
>>> +			case SYSCTL_TYPE____CTL_STR:
>>>  				req[ri].type = CTL_STR(MAX_STR_CONF_LEN);
>>>  				flags |= op == CTL_READ && !strcmp(devconfs[i], "stable_secret")
>>>  					? CTL_FLAGS_READ_EIO_SKIP : 0;
>>> @@ -230,10 +231,10 @@ static int net_conf_op(char *tgt, SysctlEntry **conf, int n, int op, char *proto
>>>  		/* (un)mark (non-)existing sysctls in image */
>>>  		for (i = 0; i < ri; i++)
>>>  			if (req[i].flags & CTL_FLAGS_HAS) {
>>> -				if (CTL_TYPE(rconf[i]->type) == CTL_32)
>>> +				if (rconf[i]->type == SYSCTL_TYPE__CTL_32)
>>>  					rconf[i]->has_iarg = true;
>>>  			} else {
>>> -				if (CTL_TYPE(rconf[i]->type) == __CTL_STR)
>>> +				if (rconf[i]->type == SYSCTL_TYPE____CTL_STR)
>>>  					rconf[i]->sarg = NULL;
>>>  			}
>>>  	}
>>> @@ -380,9 +381,9 @@ static int dump_one_netdev(int type, struct ifinfomsg *ifi,
>>>  		sysctl_entry__init(&confs6[i]);
>>>  		netdev.conf6[i] = &confs6[i];
>>>  		if (strcmp(devconfs6[i], "stable_secret")) {
>>> -			netdev.conf6[i]->type = CTL_32;
>>> +			netdev.conf6[i]->type = SYSCTL_TYPE__CTL_32;
>>>  		} else {
>>> -			netdev.conf6[i]->type = __CTL_STR;
>>> +			netdev.conf6[i]->type = SYSCTL_TYPE____CTL_STR;
>>>  			netdev.conf6[i]->sarg = stable_secret;
>>>  		}
>>>  	}
>>> @@ -1179,11 +1180,11 @@ static int dump_netns_conf(struct cr_imgset *fds)
>>>  		netns.def_conf6[i] = &def_confs6[i];
>>>  		netns.all_conf6[i] = &all_confs6[i];
>>>  		if (strcmp(devconfs6[i], "stable_secret")) {
>>> -			netns.def_conf6[i]->type = CTL_32;
>>> -			netns.all_conf6[i]->type = CTL_32;
>>> +			netns.def_conf6[i]->type = SYSCTL_TYPE__CTL_32;
>>> +			netns.all_conf6[i]->type = SYSCTL_TYPE__CTL_32;
>>>  		} else {
>>> -			netns.def_conf6[i]->type = __CTL_STR;
>>> -			netns.all_conf6[i]->type = __CTL_STR;
>>> +			netns.def_conf6[i]->type = SYSCTL_TYPE____CTL_STR;
>>> +			netns.all_conf6[i]->type = SYSCTL_TYPE____CTL_STR;
>>>  			netns.def_conf6[i]->sarg = def_stable_secret;
>>>  			netns.all_conf6[i]->sarg = all_stable_secret;
>>>  		}
>>>
>>
>