Re: [LTP] [PATCH v1] Port `getxattr01.c` to new test API

[Marius Kittler <mkittler@suse.de>]

Am Freitag, 15. September 2023, 16:10:11 CEST schrieb Andrea Cervesato via 
ltp:
> > +
> > +	/* create test file and set xattr */
> > +	struct test_case *tc = &tcases[i];
> > +	snprintf(tc->fname, BUFSIZ, "getxattr01testfile-%u", i);
> > +	int fd = SAFE_CREAT(tc->fname, 0644);
> > +	SAFE_CLOSE(fd);
> > +	TEST(setxattr(tc->fname, XATTR_TEST_KEY, XATTR_TEST_VALUE,
> > +				  strlen(XATTR_TEST_VALUE), 
XATTR_CREATE));
> 
> Here you can use TST_EXP_PASS instead of TEST
> 

I wouldn't know how. If this function fails with ENOTSUP specifically, then the 
result should be TCONF. I don't think using TST_EXP_PASS allows that. 
Additionally, if this function fails the remaining tests for this iteration 
should be skipped.

> > +
> > +	/* read xattr back */
> > +	TEST(getxattr(tc->fname, tc->key, tc->value, tc->size));
> > +	if (TST_ERR == tc->exp_err) {
> > +		tst_res(TPASS | TTERRNO, "expected getxattr() return 
code");
> > +	} else {
> > +		tst_res(TFAIL | TTERRNO, "unexpected getxattr() 
return code"
> > +				" - expected errno %d", tc-
>exp_err);
> > 
> >   	}
> 
> Also here you can use TST_EXP_PASS instead of TEST

Also here I wouldn't know how that is possible. There are test cases where an 
error is actually expected.

> > 
> > -	tst_brkm(TCONF, NULL, "<sys/xattr.h> does not exist.");
> > +#ifdef HAVE_SYS_XATTR_H
> > +	char *cwd = SAFE_GETCWD(NULL, 0);
> > +	workdir = SAFE_MALLOC(strlen(cwd) + strlen(MNTPOINT) + 2);
> > +	sprintf(workdir, "%s/%s", cwd, MNTPOINT);
> > +	free(cwd);
> 
> Here you can just SAFE_TOUCH the file if you aim to create one.
> 

Using SAFE_TOUCH() makes sense but I guess the file creation needs to be in 
run() because a file needs to be created per mount. Additionally, one then had 
to create a loop for all test cases.

> In your case, to understand LTP API a bit better, I would take
> fxgetattr01 as reference, more or less.

That test seems very similar, indeed. Do you want me to follow some patterns 
from there specifically? Note that this test also uses just "TEST(…)" similarly 
to my code.

It does a simplification by using SAFE_FSETXATTR and here I could use 
SAFE_SETXATTR. However, then we would lose the ENOTSUP case which I'm not sure 
we want/should lose (especially since I'm mainly porting/refactoring it might 
not be desirable to remove behavior at the same time).



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