Message-ID: <2127107.mNy2iA5WyI@wuerfel>
Date:	Fri, 04 Mar 2016 22:07:59 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	Gianfranco Costamagna <locutusofborg@...ian.org>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"trivial@...nel.org" <trivial@...nel.org>
Subject: Re: [PATCH] ARM: LLVMLinux: Change "extern inline" to "static inline" in mpi-inline.h and mpi-internal.h

On Friday 04 March 2016 20:48:56 Gianfranco Costamagna wrote:

> >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? :)

Can you be more specific about which compiler you use?

I had seen this problem with prerelease gcc-5.0 versions, but
I don't see it with 5.1.1, 5.2.1 or 5.3.1.

> 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 think this part was fine, only the subject line got wrapper (correctly).

The problem was really starting the email with text that should have gone
below the --- line that separate the changelog text from the additional
information.

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

That would have worked.

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

As a rule, the first signoff should always match the author, and the last
signoff should match the submitter.

If you change the patch enough to put your own name in the author field,
you should then drop my Signed-off line and instead mention me in the
changelog describing that where you found the original patch.

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

Ok, cool. Looking forward to more patches then.

	Arnd

Powered by blists - more mailing lists