Re: [LTP] [PATCH v11 1/2] lib: Extend tst_assert_ulong() with enum flags

[Andrea Cervesato via ltp <ltp@lists.linux.it>]

H Wei,

I was re-checking the whole patch-set and it looks a bit overengineered
from my point of view. A few points below.

> +enum tst_assert_flags {
> +	TST_ASSERT_SATURATED_INT = 1,
> +	TST_ASSERT_TRUNC_32BIT   = 2,
> +};

Adding new flags for the whole LTP core library trying to fix a single
test is not a good idea in general. Also, TST_ASSERT_TRUNC_32BIT is
probably not needed.

> +	safe_file_scanf(file, lineno, NULL, path, "%llu", &sys_val_64);

This is the real fix. Read below.

> +
> +	if (flags & TST_ASSERT_SATURATED_INT) {
> +		if (sys_val_64 > (unsigned long long)INT_MAX)
> +			expected_val = (unsigned long)INT_MAX;
> +		else
> +			expected_val = (unsigned long)sys_val_64;
> +	} else if (flags & TST_ASSERT_TRUNC_32BIT) {

In 32bit compat mode `unsigned long` becomes 32bit, so we can simply do:

void tst_assert_ulong(const char *file, const int lineno, const char *path, unsigned long val)
{
	unsigned long long sys_val;

	safe_file_scanf(file, lineno, NULL, path, "%llu", &sys_val);

	if (val == (unsigned long)sys_val) {
		tst_res_(file, lineno, TPASS, "%s = %lu", path, val);
		return;
	}

	tst_res_(file, lineno, TFAIL, "%s != %lu got %lu",
		path, val, (unsigned long)sys_val);
}

And then, inside the test:

	if (tst_is_compat_mode() && info.shmmax == INT_MAX)
		tst_res(TPASS, "shmmax clamped to INT_MAX in compat mode");
	else
		TST_ASSERT_ULONG(PATH_KERN_SHMMAX, info.shmmax);

	TST_ASSERT_ULONG(PATH_KERN_SHMMNI, info.shmmni);
	TST_ASSERT_ULONG(PATH_KERN_SHMALL, info.shmall);


Please verify if this is working anyway. We can probably fix the
whole patch-set with a couple of lines instead of defining
redundant flags and specific code for a single test.

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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