On Wed, Feb 07, 2024 at 10:22:48PM +0000, Al Viro wrote:
> One way to do that would be to replace d_name with
> union {
> const struct qstr d_name;
> struct qstr __d_name;
> };
> and let the the places that want to modify it use __d_name.
> Tempting, but the thing that makes me rather nervous about this
> approach is that such games with unions are straying into
> the nasal demon country (C99 6.7.3[5]), inviting the optimizers
> to play. Matt Wilcox pointed out that mainline already has
> i_nlink/__i_nlink pair, but... there are fewer users of those
> and the damage from miscompiles would be less sensitive.
> Patch along those lines would be pretty simple, though.
You're referring to this, I assume:
If an attempt is made to modify an object defined with
a const-qualified type through use of an lvalue with
non-const-qualified type, the behavior is undefined
I'm not sure that's relevant. As I see it, we're defining two objects,
one const-qualified and one not. The union specifies that they share
the same storage ("a union is a type consisting of a sequence of members
whose storage overlap").
I see 6.7.3 as saying "even if you cast away the const from d_name,
modifyig d_name is undefined", but you're not modifying d_name,
you're modifying __d_name, which just happens to share storage with
d_name.
I think 6.7.2.1[14] is more likely to cause us problems:
The value of at most one of the members can be stored in a union
object at any time.
>From that the compiler can assume that if it sees a store to __d_name
that accesses to d_name are undefined? Perhaps? My brain always starts
to hurt when people start spec-lawyering.
> * add an inlined helper,
> static inline const struct qstr *d_name(const struct dentry *d)
> {
> return &d->d_name;
> }
I'm in no way opposed to this solution. I just thought that the i_nlink
solution might also work for you (and if the compiler people say the
i_nlink solution is actually not spec compliant, then I guess we can
adopt an i_nlink_read() function).
