gnrc_ipv6_nib: remove parenthesis from boolean configs

[jia200x]

Contribution description

This PR removes all parenthesis from boolean configurations of GNRC IPv6 NIB.
These parenthesis doesn't allow the usage of the IS_ACTIVE macro, and when these values are exposed in Kconfig they won't have a parenthesis anyway.

This issue was detected by Murdock in #13226

Testing procedure

Murdock tests should be enough

Issues/PRs references

[miri64]

Isn't this against our coding conventions?

[leandrolanzieri]

Isn't this against our coding conventions?

Hmm I could not find a reference to this. What I see is section 12.4 in Linux's coding style, is:

4. forgetting about precedence: macros defining constants using expressions
   must enclose the expression in parentheses. Beware of similar issues with
   macros using parameters.

	#define CONSTANT 0x4000
	#define CONSTEXP (CONSTANT | 3)

[leandrolanzieri]

Changes look good to me. In this case all macros are boolean configurations, and it should not hurt to remove parentheses, as they don't contain complex expressions. Also, allows us to use IS_ACTIVE like intended in #13226. From my side ACK.

@miri64 further comments?

[miri64]

@miri64 further comments?

Well, I assumed until now, that definess should always have parenthesis around the value (that's why I do it all the time), so I'd like to have some clarity on that before merging this. Maybe I was just assuming it, because in the past @OlegHahm was quite insistent on explicit parenthesization.

[miri64]

Also: shouldn't IS_ACTIVE() be able to work with that?

[leandrolanzieri]

Also: shouldn't IS_ACTIVE() be able to work with that?

The doc shows that parenthesis are not supported

[miri64]

Also: shouldn't IS_ACTIVE() be able to work with that?

The doc shows that parenthesis are not supported

What is the reason for this? Preprocessor restrictions?

[leandrolanzieri]

Also: shouldn't IS_ACTIVE() be able to work with that?

The doc shows that parenthesis are not supported

What is the reason for this? Preprocessor restrictions?

Because of the way it works, it pastes the 1 with a prefix to generate a macro:

RIOT/core/include/kernel_defines.h

Lines 186 to 197 in 7943187

	/**
	* @cond INTERNAL
	*/
	/* Here a prefix "__PREFIX_WHEN_" is added to the macro. So if it was a 1 we
	* have "__PREFIX_WHEN_1", and if it was not defined we have "__PREFIX_WHEN_".
	*/
	#define __is_active(val) ___is_active(__PREFIX_WHEN_##val)
	/* With this placeholder we turn the original value into two arguments when the
	* original value was defined as 1 (note the comma).
	*/
	#define __PREFIX_WHEN_1 0,

Maybe there could be a way to strip the parentheses before evaluating, but I'm not really sure how that would work :/

[miri64]

Maybe there could be a way to strip the parentheses before evaluating, but I'm not really sure how that would work :/

Have to research that myself, but IIRC there is a macro that reduces a marco to this final value. Can't we use that?

[jia200x]

Well, I assumed until now, that definess should always have parenthesis around the value (that's why I do it all the time), so I'd like to have some clarity on that before merging this. Maybe I was just assuming it, because in the past @OlegHahm was quite insistent on explicit parenthesization.

For expressions it totally makes sense:

#define A 2+2
#define B (2+2)

printtf("%i", A*2); /* prints 6 */
printtf("%i", B*2); /* prints 8 */

However for booleans, there's no case where this might happen. And it still aligns with Linux conventions.

[miri64]

Mh, but couldn't this lead to cases that through some inattentive combination of macros

#define FOO 1
#define BAR 0

could become 10 when FOO comb BAR, where comb is the combination operation?

[jia200x]

could become 10 when FOO comb BAR, where comb is the combination operation?

Hmmm but in this case (1)(0) would also produce an error.

[miri64]

could become 10 when FOO comb BAR, where comb is the combination operation?

Hmmm but in this case (1)(0) would also produce an error.

Exactly, which is preferable over using an accidentally wrongly created value.

[miri64]

On the other hand... This seems to be an old discussion (the linked one comes to the conclusion "value defines without parenthesis are actually safer). So given that we have differing conclusions and I don't remember exactly the details, why I am following this cargo-cult, I won't block this anymore (actually I never did, but I won't hit the green button ;-))

[leandrolanzieri]

So given that we have differing conclusions and I don't remember exactly the details, why I am following this cargo-cult, I won't block this anymore (actually I never did, but I won't hit the green button ;-))

Ok then, let's go with this.