Re: [LTP] [PATCH v2 3/3] umount2/umount2_03.c: add new test

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> +static void umount2_verify(void)
> +{
> +	SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
> +	mount_flag = 1;
> +
> +	TEST(umount2(SYMLINK, UMOUNT_NOFOLLOW));
> +
> +	if (TEST_RETURN == 0 || TEST_ERRNO != EINVAL) {
> +		tst_resm(TFAIL | TTERRNO,
> +			"umount2(2) UMOUNT_NOFOLLOW flag "
> +			"performed abnormally "
> +			"expected error = %d : %s",
> +			EINVAL, strerror(EINVAL));

Let's keep the messages short and to the point, what about:


tst_resm(TFAIL | TTERRNO,
         "umount2('symlink', UMOUNT_NOFOLLOW) failed unexpectedly, expected EINVAL");


Also it would be better if if we had separate case for the when the call
succeeded something as:

tst_resm(TFAIL, "umount2('symlink', UMOUNT_NOFOLLOW) "
         "succeeded unexpectedly");

> +		if (TEST_RETURN == 0)
> +			mount_flag = 0;
> +		goto EXIT;
> +	}
> +
> +	TEST(umount2(MNTPOINT, UMOUNT_NOFOLLOW));
> +
> +	if (TEST_RETURN != 0) {
> +		tst_resm(TFAIL | TTERRNO, "umount2(2) Failed");
> +		goto EXIT;
> +	}
> +
> +	mount_flag = 0;
> +
> +	tst_resm(TPASS, "umount2(2) Passed");
> +
> +EXIT:
> +	if (mount_flag) {
> +		SAFE_UMOUNT(cleanup, MNTPOINT);
> +		mount_flag = 0;
> +	}
> +}


Again, these are two testcases you should print two PASS/FAIL for each of
them. What about you split the verify funciton into two
umount2_verify_failure and umount2_verify_success and mount and umout
the device in each of them. That way we can make the code flow more
straightforward.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list