Thread (24 messages) 24 messages, 4 authors, 2021-01-27

Re: [PATCH v4 8/8] mm: Mark anonymous struct field of 'struct vm_fault' as 'const'

From: Nick Desaulniers <hidden>
Date: 2021-01-21 19:26:37
Also in: linux-mm, lkml

On Thu, Jan 21, 2021 at 5:11 AM Will Deacon [off-list ref] wrote:
On Wed, Jan 20, 2021 at 11:02:06AM -0800, Linus Torvalds wrote:
quoted
On Wed, Jan 20, 2021 at 10:27 AM Nick Desaulniers
[off-list ref] wrote:
quoted
Is there a difference between: [ const unnamed struct and individual const members ]
Semantically? No.

Syntactically the "group the const members together" is a lot cleaner,
imho. Not just from a "just a single const" standpoint, but from a
"code as documentation" standpoint.

But I guess to avoid the clang issue, we could do the "mark individual
fields" thing.
I'd prefer to wait until the bug against LLVM has been resolved before we
try to work around anything. Although I couldn't find any other examples
like this in the kernel, requiring all of the member fields to be marked as
'const' still feels pretty fragile to me; it's only a matter of time before
new non-const fields get added, at which point the temptation for developers
to remove 'const' from other fields when it gets in the way is pretty high.
What's to stop a new non-const field from getting added outside the
const qualified anonymous struct?
What's to stop someone from removing const from the anonymous struct?
What's to stop a number of callers from manipulating the structure
temporarily before restoring it when returning by casting away the
const?

Code review.

Using a non-toolchain-portable solution certainly could be considered
more fragile.

It's always possible that the resolution is the C standards body goes
the C++ route, at which point GCC would be forced to address this and
potentially change behavior.  Kind of like how people avoid going to
court since things are never guaranteed to work out in their favor.
None of this is bullet-proof, of course, but if clang ends up emitting a
warning (even if it's gated behind an option) then I think we're in a good
place.
quoted
(It turns out that sparse gets this wrong too, so it's not just clang).
Adding Luc, as hopefully that's fixable.

Will


--
Thanks,
~Nick Desaulniers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help