[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHk-=wi2VOPoweqnDhxXKJ9fcLQzkV1oEDjteV=z-C7KXrpomg@mail.gmail.com>
Date: Thu, 5 Sep 2019 09:20:15 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc: Nick Desaulniers <ndesaulniers@...gle.com>,
Will Deacon <will@...nel.org>,
Josh Poimboeuf <jpoimboe@...hat.com>,
"Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>,
"David S. Miller" <davem@...emloft.net>,
Paul Burton <paul.burton@...s.com>,
Sedat Dilek <sedat.dilek@...il.com>,
Linux List Kernel Mailing <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] compiler-attributes for v5.3-rc8
On Wed, Sep 4, 2019 at 11:18 AM Miguel Ojeda
<miguel.ojeda.sandonis@...il.com> wrote:
>
> I was going to send this for 5.4 since it is not that trivial, but since
> you are doing an -rc8, and it fixes an oops, please consider pulling it.
I looked at this, and while it seems safe, I end up worrying.
Macro stringification isn't entirely obvious, and an unquoted string
could become corrupted if the stringification ends up not happening
immediately.
It does seem safe just because we do
#define __section(S) __attribute__((__section__(#S)))
but I had to go _check_ that we do, because it wouldn't have been safe
if there had been another level of macro expansion, because then the
argument in turn could have been expanded before it was stringified.
So sometimes you actually _want_ to pass in a string to be
stringified, because it's safer. I realize it then gets string-quoted,
but this has worked for gcc. Even if I suspect nobody really _thought_
about it.
So I'm not unhappy about the patch, but it's the kind of thing I'd
really prefer not to do at this stage.
Particularly since it seems to do other things too than just fix
double quoting. As far as I can tell, it doesn't just fix double
string quoting, it changes a lot of singly-quoted strings to use the
macro and unquotes them, ie
- __attribute__((__section__(".arch.info.init"))) = { \
+ __section(.arch.info.init) = { \
doesn't actually "fix" anything that I can see, it just uses the simpler form.
Linus
Powered by blists - more mailing lists