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

[Pavel Tikhomirov]

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.

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

>
>>  			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;
>>  		}
>>
>

-- 
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.