On Tue, 2022-04-19 at 14:30 +0100, David Howells wrote: > Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > > On Tue, 2022-04-19 at 14:22 +0100, David Howells wrote: > > > Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > > > > > > if ((flags & AT_STATX_SYNC_TYPE) == (AT_STATX_DONT_SYNC|AT_STATX_FORCE_SYNC)) > > > > > > You can't do that. DONT_SYNC and FORCE_SYNC aren't bit flags - they're an > > > enumeration in a bit field. There's a reserved value at 0x6000 that doesn't > > > have a symbol assigned. > > > > > > > Right, but nothing prevents you from setting both flags at the same > > time. How should we interpret such a request? > > A good question without a necessarily right answer. > > Possibly we should do: > > #define AT_STATX_SYNC_TYPE 0x6000 /* Type of synchronisation required from statx() */ > #define AT_STATX_SYNC_AS_STAT 0x0000 /* - Do whatever stat() does */ > #define AT_STATX_FORCE_SYNC 0x2000 /* - Force the attributes to be sync'd with the server */ > #define AT_STATX_DONT_SYNC 0x4000 /* - Don't sync attributes with the server */ > +#define AT_STATX_SYNC_RESERVED 0x6000 > > and give EINVAL if we see the reserved value. But also these values can be > considered a hint, so possibly we should just ignore the reserved value. Oh > for fsinfo()... > > David > That was what the code (pre-patch) did. If someone set DONT_SYNC|FORCE_SYNC, it would just ignore FORCE_SYNC. It's not ideal, but I suppose we're within our rights to prefer either behaviour in that case if someone sets both flags. In hindsight, setting both should have probably caused the syscall to throw back -EINVAL, but changing that now is probably a bit dangerous. -- Jeff Layton <jlayton@xxxxxxxxxx>
