Re: [LTP] [PATCH 1/4] fanotify14: Print human-readable test case flags

[Petr Vorel <pvorel@suse.cz>]

> On 13. 10. 22 23:36, Petr Vorel wrote:
> > Hi Martin,

> > > It's hard to tell which test case is failing from the current fanotify14
> > > output. Print test case flags to make failure analysis easier.

> > > Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> > > ---
> > >   .../kernel/syscalls/fanotify/fanotify14.c     | 194 ++++++++++--------
> > >   1 file changed, 108 insertions(+), 86 deletions(-)

> > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c
> > > index 594259ccf..ee42aaf68 100644
> > > --- a/testcases/kernel/syscalls/fanotify/fanotify14.c
> > > +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c
> > > @@ -38,6 +38,8 @@
> > >   #define INODE_EVENTS (FAN_ATTRIB | FAN_CREATE | FAN_DELETE | FAN_MOVE | \
> > >   		      FAN_DELETE_SELF | FAN_MOVE_SELF)

> > > +#define FLAGS_DESC(flags) (flags), (#flags)
> > +1 for add ing description. But macro like this gets false positive in make
> > check:

> > fanotify14.c:41: ERROR: Macros with complex values should be enclosed in parentheses

> > Also quite recently, in dbb9db6ec ("syscalls/fanotify09: Make test case
> > definitions more readable") was single test migrated to use
> > .foo = value, .bar = value struct setup. This is about source code readability,
> > you aim for test output readability, IMHO both is important.

> I can't use that approach here because I'm using the macro to initialize 3
> different pairs of attributes in the same structure. What I could do is
> change the flags/desc pairs into a nested struct of
> {unsigned long, const char *} and the macro would change to
> #define FLAGS_DESC(flags) {(flags), (#flags)}

+1, but it's not important. Amir's comments are what is important.

> > > @@ -155,8 +169,14 @@ static void do_test(unsigned int number)
> > >   		return;
> > >   	}

> > > -	TST_EXP_FD_OR_FAIL(fanotify_fd = fanotify_init(tc->init_flags, O_RDONLY),
> > > -			   !tc->mask && tc->expected_errno ? tc->expected_errno : 0);
> > TST_EXP_FD_OR_FAIL was added only to be used by fanotify tests.
> > What's wrong with it?

> I got a headache trying to figure out when this call was expected to pass
> and when it was expected to fail. The more verbose version below is far
> easier to understand.

Sure, thanks for an explanation.

Kind regards,
Petr

> > > +	if (!tc->mask && tc->expected_errno) {
> > > +		TST_EXP_FAIL(fanotify_init(tc->init_flags, O_RDONLY),
> > > +			tc->expected_errno);
> > > +	} else {
> > > +		TST_EXP_FD(fanotify_init(tc->init_flags, O_RDONLY));
> > > +	}
> > > +
> > > +	fanotify_fd = TST_RET;

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp