[LTP] [PATCH v5 4/7] syscalls/statx01: Relax mnt_id test condition

[Yang Xu (Fujitsu)]

Hi Eric


> Hi Yang,
> 
> On Tue, May 09, 2023 at 05:14:05PM +0800, Yang Xu wrote:
>> Before this patch, we test stx_mnt_id only when glibc's statx struct has
>> this member. Now, if glibc miss this filed, we will use __spare2[0], see
>> url[1]. If glibc miss statx struct, we will use ltp owner definition.
> 
> I don't think this is the right approach either.  Actually, this new proposal is
> arguably worse than leaving the problem unsolved.  The problem with this new
> proposal is that the fields in struct statx whose names are prefixed with
> "__spare" were never intended to be used directly.  Their names can change from
> one kernel version to the next, and they can change to a different offset and/or
> size while keeping the same name.  That can result in breakages that only
> reproduce on very specific versions of <sys/stat.h>.
> 

Ok, understood.

> As I've said several times, the proper way to ensure that the tests can be built
> even when the system struct statx doesn't contain all the needed fields is to
> use the LTP struct statx whenever the system one does not fully suffice.
> 
> One way to do that would be to make the tests use the struct via a different
> name, such as struct ltp_statx, and define that to either the system statx or
> the LTP statx depending on which one has the right definition.

Yes, use ltp_statx struct is a better sloution.

> 
> It might also be possible to #define statx to achieve the same effect (though
> maybe that would cause a collision with the function statx()...)

Indeed, it will report warning as below:
note: expected ‘struct statx * restrict’ but argument is of type ‘struct 
ltp_statx *’\

I change code as below:
+#if !defined(HAVE_STRUCT_STATX) ||  \
+    !defined(HAVE_STRUCT_STATX_MNT_ID) || \
+    !defined(HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN)
+struct ltp_statx {
         /* 0x00 */
         uint32_t        stx_mask;
         uint32_t        stx_blksize;
@@ -102,6 +104,8 @@ struct statx {
         uint64_t        __spare3[12];
         /* 0x100 */
  };
+#else
+#define ltp_statx statx;



> 
> Anyway, I don't want to derail the STATX_DIOALIGN test too much, so if you're
> having a lot of trouble solving this problem properly, I'm okay with just
> adding the STATX_DIOALIGN test without it solved for now...

In fact, I just want to leave run or not run this case on glibc header 
ie  I do in testing mnt_id[1]. It is simple and clean alough it will 
miss some overage if using a custom new kernel and old glibc. In other 
situation(ie various offical linux distributions), it should work well.


So I will send a v6 that simply just use #ifndef 
HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN.

[1]https://github.com/linux-test-project/ltp/commit/0578cfb3dc175426aff516025b3ca76103e5f551

Best Regards
Yang Xu
> 
> - Eric