All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Vorel <pvorel@suse.cz>
To: Wei Gao <wegao@suse.com>
Cc: ltp@lists.linux.it
Subject: Re: [LTP] [PATCH v11 1/2] lib: Extend tst_assert_ulong() with enum flags
Date: Wed, 16 Sep 2026 13:33:39 +0200	[thread overview]
Message-ID: <20260916113339.GA1756857@pevik> (raw)
In-Reply-To: <ap5n5QV5xneoAFYm@autotest-wegao.qe.prg2.suse.org>

> On Fri, Sep 04, 2026 at 08:51:06AM +0000, Andrea Cervesato wrote:
> > 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.

> But this is follow Cyril's suggestion:
> https://lore.kernel.org/ltp/aJm5SBOaRoe1e0PB@yuki.lan/

FYI the informal rule Cyril announced few weeks ago "at least two tests needs
it" (we talked about .max_kver member of struct tst_test, I guess this would
more or less applied to library API code.

When I acked this in v9 [1] (after suggesting to use enum instead of #define in
v8 [2]) I expected it'd be actually needed in more tests, based on Cyril's
suggestion.

Anyway, Dan Carpenter mentioned [3] kernel commit af7c693f1460
("Cap shmmax at INT_MAX in compat shminfo") which does 

+       if (smi->shmmax > INT_MAX)
+               smi->shmmax = INT_MAX;

[1] https://lore.kernel.org/ltp/20260902090418.GA910767@pevik/
[2] https://lore.kernel.org/ltp/20260828092602.GA715959@pevik/
[3] https://lore.kernel.org/ltp/aJYBSmcQpbHUvOqW@stanley.mountain/

Searching in kernel git

$ git grep -A2 'if.* > INT_MAX' |grep ' = INT_MAX' 

I found only:

block/bfq-iosched.c
drivers/usb/class/usbtmc.c
drivers/xen/pvcalls-front.c
fs/ext4/inode.c
include/net/netfilter/nf_conntrack_core.h
ipc/shm.c
kernel/time/itimer.c
lib/vsprintf.c
net/mptcp/protocol.c
sound/core/oss/pcm_oss.c
tools/lib/bpf/ringbuf.c

IMHO none of them uses export data truncated by INT_MAX to sysfs.

> > > +	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);
> > 	}

> > 	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);

Looks like quite elegant solution, give we really need it just for a single
test.

> > 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.

Yes it works.

=> I vote to merge Andrea's solution instead.
@Cyril @Li WDYT?

Kind regards,
Petr

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

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

  reply	other threads:[~2026-09-16 11:34 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-03  9:52 [LTP] [PATCH v11 0/2] shmctl03: Fix 32-bit compat mode failure Wei Gao via ltp
2026-09-03  9:52 ` [LTP] [PATCH v11 1/2] lib: Extend tst_assert_ulong() with enum flags Wei Gao via ltp
2026-09-03 12:00   ` [LTP] " linuxtestproject.agent
2026-09-04  8:51   ` [LTP] [PATCH v11 1/2] " Andrea Cervesato via ltp
2026-09-07  7:29     ` Wei Gao via ltp
2026-09-16 11:33       ` Petr Vorel [this message]
2026-09-17  7:42         ` Li Wang
2026-09-23 11:17     ` Cyril Hrubis
2026-09-03  9:52 ` [LTP] [PATCH v11 2/2] shmctl03: Fix 32-bit compat mode failure Wei Gao via ltp

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260916113339.GA1756857@pevik \
    --to=pvorel@suse.cz \
    --cc=ltp@lists.linux.it \
    --cc=wegao@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.