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

[Gianfranco Costamagna]

Hi



>Close, but not quite right.

>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.


wonderful, sure, in the next merge window is good.
But there is a little difference between your patch and mine.

In your case they were build warnings, now with gcc-5 as default
(e.g. Debian, Ubuntu), they are errors, so I would give the patch
merge a speedup...

but who am I to give suggestions to kernel folks? :)

I can use the patch locally, and wait for the next release (actually
I'm using it on iMX6 machines, so I have to backport this kind of patches
anyway)
>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).


I did a git format-patch -1 commitid, from the source tree.
The mail client (Thunderbird) has been configured with the documentation 

in the source tree, but I think it failed to not wrap the lines :)


(I also have used git send-email or something similar some months ago, maybe
I could have just used it)

>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.


I know really good git, but I left it that way, because actually the patch was different
from your one, and I don't want people blaming you instead of me.

(not sure if the right approach, I thought the signoff was something nice, but
probably I agree it is even worse)

>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.


true story, bad copy paste from commits
aeea3592a13bf12861943e44fc48f1f270941f8d
76ae03828756bac2c1fa2c7eff7485e5f815dbdb

I'll try to make it right the next time!

thanks for the explanation, I patch kernel almost daily, I hope to contribute to it
in the near future.

cheers,

Gianfranco