[OpenWrt-Devel] [PATCH v2 2/2] procd: apply official kernel clang-format style

[Daniel Golle]

I don't think adding formatter-information into the code makes sense
at this stage, as this is meant for exeptions from an otherwise applied
style. Here, obviously clang-format defintions don't reflect the
current style.

I will show some examples:

On Tue, May 14, 2019 at 05:56:11PM +0200, Paul Spooren wrote:
> using clang-format-9

> @@ -209,35 +207,39 @@ static void rcrespawn(struct init_action *a)
>  	fork_worker(a);
>  }
>  
> -static struct init_handler handlers[] = {
> -	{
> -		.name = "sysinit",
> -		.cb = runrc,
> -	}, {
> -		.name = "shutdown",
> -		.cb = runrc,
> -	}, {
> -		.name = "askfirst",
> -		.cb = askfirst,
> -		.multi = 1,
> -	}, {
> -		.name = "askconsole",
> -		.cb = askconsole,
> -		.multi = 1,
> -	}, {
> -		.name = "respawn",
> -		.cb = rcrespawn,
> -		.multi = 1,
> -	}, {
> -		.name = "askconsolelate",
> -		.cb = askconsole,
> -		.multi = 1,
> -	}, {
> -		.name = "respawnlate",
> -		.cb = rcrespawn,
> -		.multi = 1,
> -	}
> -};
> +static struct init_handler handlers[] = { {
> +						  .name = "sysinit",
> +						  .cb = runrc,
> +					  },
> +					  {
> +						  .name = "shutdown",
> +						  .cb = runrc,
> +					  },
> +					  {
> +						  .name = "askfirst",
> +						  .cb = askfirst,
> +						  .multi = 1,
> +					  },
> +					  {
> +						  .name = "askconsole",
> +						  .cb = askconsole,
> +						  .multi = 1,
> +					  },
> +					  {
> +						  .name = "respawn",
> +						  .cb = rcrespawn,
> +						  .multi = 1,
> +					  },
> +					  {
> +						  .name = "askconsolelate",
> +						  .cb = askconsole,
> +						  .multi = 1,
> +					  },
> +					  {
> +						  .name = "respawnlate",
> +						  .cb = rcrespawn,
> +						  .multi = 1,
> +					  } };

Now this is a bit annoying (unnessecarily high indention level) but
still baerable.

> diff --git a/preload.h b/preload.h
> index 5e663ac..5a09db0 100644
> --- a/preload.h
> +++ b/preload.h
> @@ -18,39 +18,24 @@
>  #endif
>  
>  #ifndef attribute_unused
> -#define attribute_unused __attribute__ ((unused))
> +#define attribute_unused __attribute__((unused))
>  #endif
>  typedef int (*main_t)(int, char **, char **);
>  
>  typedef int (*start_main_t)(main_t main, int, char *__unbounded *__unbounded,
> -			ElfW(auxv_t) *,
> -			__typeof (main),
> -			void (*fini) (void),
> -			void (*rtld_fini) (void),
> -			void *__unbounded stack_end);
> -
> -int __libc_start_main(main_t main,
> -			int argc,
> -			char **argv,
> -			ElfW(auxv_t) *auxvec,
> -			__typeof (main) init,
> -			void (*fini) (void),
> -			void (*rtld_fini) (void),
> -			void *stack_end);
> -
> -
> -typedef void (*uClibc_main)(main_t main,
> -			int argc,
> -			char **argv,
> -			void (*app_init)(void),
> -			void (*app_fini)(void),
> -			void (*rtld_fini)(void),
> -			void *stack_end attribute_unused);
> -
> -void __uClibc_main(main_t main,
> -			int argc,
> -			char **argv,
> -			void (*app_init)(void),
> -			void (*app_fini)(void),
> -			void (*rtld_fini)(void),
> -			void *stack_end attribute_unused);
> +			    ElfW(auxv_t) *, __typeof(main), void (*fini)(void),
> +			    void (*rtld_fini)(void),
> +			    void *__unbounded stack_end);
> +
> +int __libc_start_main(main_t main, int argc, char **argv, ElfW(auxv_t) * auxvec,
> +		      __typeof(main) init, void (*fini)(void),
> +		      void (*rtld_fini)(void), void *stack_end);
> +
> +typedef void (*uClibc_main)(main_t main, int argc, char **argv,
> +			    void (*app_init)(void), void (*app_fini)(void),
> +			    void (*rtld_fini)(void),
> +			    void *stack_end attribute_unused);
> +
> +void __uClibc_main(main_t main, int argc, char **argv, void (*app_init)(void),
> +		   void (*app_fini)(void), void (*rtld_fini)(void),
> +		   void *stack_end attribute_unused);

Having each parameter in a single line may be desirable for
functions having many parameters and, like here, when defining
function-pointer types.

> diff --git a/signal.c b/signal.c
> index 9974153..df2254c 100644
> --- a/signal.c
> +++ b/signal.c
> @@ -27,7 +27,7 @@ static void do_reboot(void)
>  	sleep(2);
>  	reboot(RB_AUTOBOOT);
>  	while (1)
> -	;
> +		;
>  }
>  
>  static void signal_shutdown(int signal, siginfo_t *siginfo, void *data)
> @@ -36,7 +36,7 @@ static void signal_shutdown(int signal, siginfo_t *siginfo, void *data)
>  	char *msg = NULL;
>  
>  #ifndef DISABLE_INIT
> -	switch(signal) {
> +	switch (signal) {
>  	case SIGINT:
>  	case SIGTERM:
>  		event = RB_AUTOBOOT;
> @@ -56,10 +56,8 @@ static void signal_shutdown(int signal, siginfo_t *siginfo, void *data)
>  		procd_shutdown(event);
>  }
>  
> -struct sigaction sa_shutdown = {
> -	.sa_sigaction = signal_shutdown,
> -	.sa_flags = SA_SIGINFO
> -};
> +struct sigaction sa_shutdown = { .sa_sigaction = signal_shutdown,
> +				 .sa_flags = SA_SIGINFO };

This is much uglier than it was.

>  
>  static void signal_crash(int signal, siginfo_t *siginfo, void *data)
>  {
> @@ -67,20 +65,16 @@ static void signal_crash(int signal, siginfo_t *siginfo, void *data)
>  	do_reboot();
>  }
>  
> -struct sigaction sa_crash = {
> -	.sa_sigaction = signal_crash,
> -	.sa_flags = SA_SIGINFO
> -};
> +struct sigaction sa_crash = { .sa_sigaction = signal_crash,
> +			      .sa_flags = SA_SIGINFO };

Same here.

>  
>  static void signal_dummy(int signal, siginfo_t *siginfo, void *data)
>  {
>  	ERROR("Got unexpected signal %d\n", signal);
>  }
>  
> -struct sigaction sa_dummy = {
> -	.sa_sigaction = signal_dummy,
> -	.sa_flags = SA_SIGINFO
> -};
> +struct sigaction sa_dummy = { .sa_sigaction = signal_dummy,
> +			      .sa_flags = SA_SIGINFO };

And here.

> -enum {
> -	STATE_NONE = 0,
> -	STATE_EARLY,
> -	STATE_UBUS,
> -	STATE_INIT,
> -	STATE_RUNNING,
> -	STATE_SHUTDOWN,
> -	STATE_HALT,
> -	__STATE_MAX,
> +enum { STATE_NONE = 0,
> +       STATE_EARLY,
> +       STATE_UBUS,
> +       STATE_INIT,
> +       STATE_RUNNING,
> +       STATE_SHUTDOWN,
> +       STATE_HALT,
> +       __STATE_MAX,
>  };

Here it replaces tabs with spaces...


>  static int system_board(struct ubus_context *ctx, struct ubus_object *obj,
> -                 struct ubus_request_data *req, const char *method,
> -                 struct blob_attr *msg)
> +			struct ubus_request_data *req, const char *method,
> +			struct blob_attr *msg)

...and here it does the opposite.



Maybe checkpatch.pl or existing kernel-style check tools would be a
better reference than the relatively young and only recently popular
clang-format?

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel