From: Arnd Bergmann <arnd@arndb.de>
To: Gianfranco Costamagna <locutusofborg@debian.org>
Cc: linux-kernel@vger.kernel.org, trivial@kernel.org
Subject: Re: [PATCH] ARM: LLVMLinux: Change "extern inline" to "static inline" in mpi-inline.h and mpi-internal.h
Date: Fri, 04 Mar 2016 15:16:05 +0100
Message-ID: <5693177.gLSTqHZso3@wuerfel>
User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; )
In-Reply-To: <56D99351.8080601@debian.org>
References: <56D99351.8080601@debian.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3142
Lines: 77

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@abinsula.com>
> 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@arndb.de>
> Signed-off-by: Gianfranco Costamagna <gianfranco.costamagna@abinsula.com>
> Signed-off-by: Gianfranco Costamagna <locutusofborg@debian.org>

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@arndb.de>"',
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