Re: [PATCH] ARM: LLVMLinux: Change "extern inline" to "static inline" in mpi-inline.h and mpi-internal.h

[Arnd Bergmann]

On Friday 04 March 2016 14:53:21 Gianfranco Costamagna wrote:
> Hi, this is my first contribution to the kernel code, I hope I did it right.

Close, but not quite right.

> Today I faced a gcc-5 build failure, so I fixed it (the static inline
> C99 issue)
> 
> After I found the same patch from Arnd, but I fail to see it applied to
> the kernel source code.
> 
> according to LKML [1] the patch should be already applied, but I fail to
> see it in current master.

It's currently in the crypto tree after Herbert Xu picked it up.
It is scheduled to be merged into 4.6 at the moment.

> Sending it with the format-patch style.
> 
> 
> [1] https://lkml.org/lkml/2016/2/26/459
> 
> From 820a0ad32d474adf925437811e9b61d9d8886bc9 Mon Sep 17 00:00:00 2001
> From: Gianfranco Costamagna <gianfranco.costamagna@xxxxxxxxxxxx>
> Date: Fri, 4 Mar 2016 14:16:24 +0100
> Subject: [PATCH] ARM: LLVMLinux: Change "extern inline" to "static
> inline" in
>  mpi-inline.h and mpi-internal.h

The mail is not formatted in a way that allows being imported with
'git am'. The best way to do this right is to use the headers
as they come from git format-patch directly, and start the mail with
the changelog text (and with a From: line before that).

> With compilers which follow the C99 standard (like modern versions of
> gcc and
> clang), "extern inline" does the wrong thing (emits code for an externally
> linkable version of the inline function). "static inline" is the correct
> choice
> instead.
> 
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> Signed-off-by: Gianfranco Costamagna <gianfranco.costamagna@xxxxxxxxxxxx>
> Signed-off-by: Gianfranco Costamagna <locutusofborg@xxxxxxxxxx>

Here you have kept my Signed-off-by line, but not the author attribution.
This is easy to get wrong. You probably want to change the author
field using 'git commit --amend --author="Arnd Bergmann <arnd@xxxxxxxx>"',
which will cause the correct From: line to show up when exporting it
with git send-email.

> index c65dd1b..1baca30 100644
> --- a/lib/mpi/mpi-internal.h
> +++ b/lib/mpi/mpi-internal.h
> @@ -168,19 +168,19 @@ void mpi_rshift_limbs(MPI a, unsigned int count);
>  int mpi_lshift_limbs(MPI a, unsigned int count);
> 
>  /*-- mpihelp-add.c --*/
> -mpi_limb_t mpihelp_add_1(mpi_ptr_t res_ptr, mpi_ptr_t s1_ptr,
> +static mpi_limb_t mpihelp_add_1(mpi_ptr_t res_ptr, mpi_ptr_t s1_ptr,
>  			 mpi_size_t s1_size, mpi_limb_t s2_limb);
>  mpi_limb_t mpihelp_add_n(mpi_ptr_t res_ptr, mpi_ptr_t s1_ptr,
>  			 mpi_ptr_t s2_ptr, mpi_size_t size);
> -mpi_limb_t mpihelp_add(mpi_ptr_t res_ptr, mpi_ptr_t s1_ptr, mpi_size_t
> s1_size,
> +static mpi_limb_t mpihelp_add(mpi_ptr_t res_ptr, mpi_ptr_t s1_ptr,
> mpi_size_t s1_size,
>  		       mpi_ptr_t s2_ptr, mpi_size_t s2_size);

You have marked the function as 'static' here, rather than 'static inline'
as I did in my patch. I think my version is better here, because it matches
the definition of the function, and because declaring a function as
'static' in a header file is generally a bad idea: you will get a build
warning or error if the header is included in a file that does not provide
a definition.

	Arnd