[PATCH] Add utilities to check style

Ralph Böhme slow at samba.org
Thu Apr 26 15:06:33 UTC 2018


On Wed, Apr 25, 2018 at 09:26:12PM -0400, Simo wrote:
> The easy way is to use a blank line:
> 
> 	if (((state->dhnc != NULL) && (state->dh2c != NULL)) ||
> 	    ((state->dhnc != NULL) && (state->dh2q != NULL)) ||
>             ((state->dh2c != NULL) && (state->dhnq != NULL)) ||
>              ((state->dh2q != NULL) && (state->dh2c != NULL))) {
> 
>                  /* not both are allowed at the same time */
>                  return NT_STATUS_INVALID_PARAMETER;
>          }

heavens, no. I guess I got used to the README.Coding style, so it feels quite
natural and the above just feels like two indentation errors in one line. :)

> IMHO a better way to deal with such a long statement is to use a
> MACRO instead.
> 
> #define WHATEVER_STATE_NOT_NULL(s) \
> 	(s->dhnc || s->dh2c || s->dhnc || s->dh2q || \
> 	 s->dh2c || s->dhnq || s->dh2q || s->dh2c)
> 
> then that if turns into:
> 
> 	if (WHATEVER_STATE_NOT_NULL(state)) {
> 		/* not both are allowed at the same time */
> 		return NT_STATUS_INVALID_PARAMETER;
> 	}
> 
> Which is clearly much more readable and even reusable.

Good luck inventing reusable macros for all the places with multi-line
conditional expressions:

$ git grep -P -A 5 'if \((?!.*{)'

:)

> Often the problem is that there are such long statements in the first
> place.

I guess that makes for a different README.Coding rule.

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG Key Fingerprint:           FAE2 C608 8A24 2520 51C5
                               59E4 AA1E 9B71 2639 9E46



More information about the samba-technical mailing list