On 01/12/2022 12:14, Zachary Leaf wrote:
-#define CLONE_ARGS_SIZE_VER0 64 /* sizeof first published struct */ -#define CLONE_ARGS_SIZE_VER1 80 /* sizeof second published struct */ -#define CLONE_ARGS_SIZE_VER2 88 /* sizeof third published struct */ +#define CLONE_ARGS_SIZE_VER0 /* sizeof first published struct */ \
- ((__alignof__(__kernel_uintptr_t) > __alignof__(__u64)) ? 128 : 64)
+#define CLONE_ARGS_SIZE_VER1 /* sizeof second published struct */ \
- ((__alignof__(__kernel_uintptr_t) > __alignof__(__u64)) ? 152 : 80)
+#define CLONE_ARGS_SIZE_VER2 /* sizeof third published struct */ \
- ((__alignof__(__kernel_uintptr_t) > __alignof__(__u64)) ? 160 : 88)
This is just hardcoding in the struct sizes which is fine, but just as a
That's how this struct is being versioned.
general question, would it ever be possible for __alignof__(__kernel_uintptr_t) to be different or more than 16?
If __kernel_uintptr_t is 64bits then yes, the alignment will be on 8-byte boundaries. Additionally, as __kernel_uintptr_t is defined as __u64 for !PCuABI it can be aligned on 4-byte boundaries for some 32-bit platforms, which is why this struct is forcing the alignment on it's members to be (now at least) 8 bytes.
Seems like what we've got here is just a proxy for: ((__alignof__(__kernel_uintptr_t) == 16) ? 160 : 88)
Yes, although I do prefer the verbosity here. Do you believe this is not readable and should be changed ?
In hindsight I think it's fine.
My initial concern/question here was if ever in future we might have have __kernel_uintptr_t being 32B/256-bit or something like that. Then it might be better to be explicit about what values of alignof(x) relate to what struct sizes instead of specifying it just as a relation of x > y.
In that case we've got more than 2 options and what I suggested wouldn't work either anyway and would need additional cases.
In this particular case we need to handle each size (above 64-bit) explicitly so there's no fundamental difference between either of these approaches.
I don't have a strong preference here but just one thing I wanted to note, it doesn't really make sense to look at the alignment of __kernel_uintptr_t vs __u64, as what we have in that struct is the __aligned variant of both. I think we can safely assume that __aligned means self-aligned, i.e. sizeof() == alignof(), so I would replace __alignof__ with sizeof in the tests.
Kevin