Re: [PATCH v4 8/8] mm: Mark anonymous struct field of 'struct vm_fault' as 'const'
From: Will Deacon <will@kernel.org>
Date: 2021-01-21 21:29:28
Also in:
linux-mm, lkml
On Thu, Jan 21, 2021 at 11:24:36AM -0800, Nick Desaulniers wrote:
On Thu, Jan 21, 2021 at 5:11 AM Will Deacon [off-list ref] wrote:quoted
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.
Sure, but here we are cleaning up this stuff, so I think review only gets
you so far. To me:
const struct {
int foo;
long bar;
};
clearly says "don't modify fields of this struct", whereas:
struct {
const int foo;
const long bar;
};
says "don't modify foo or bar, but add whatever you like on the end" and
that's the slippery slope. So then we end up with the eye-sore of:
const struct {
const int foo;
const long bar;
};
and maybe that's the right answer, but I'm just saying we should wait for
clang to make up its mind first. It's not like this is a functional problem,
and there are enough GCC users around that we're not exactly in a hurry.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel