public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Petr Vorel <pvorel@suse.cz>
To: Richard Palethorpe <rpalethorpe@suse.de>
Cc: ltp@lists.linux.it
Subject: Re: [LTP] [PATCH v5] statvfs01: Convert to new LTP API
Date: Thu, 22 Dec 2022 10:49:32 +0100	[thread overview]
Message-ID: <Y6QoLAfvY/W175WH@pevik> (raw)
In-Reply-To: <87a63ith0b.fsf@suse.de>

Hi Avinesh, Richie,

Generally LGTM, thanks for fixing exfat and vfat.

> > +	toolong_fname = SAFE_MALLOC(buf.f_namemax + 1);
However, length could be smaller:
Instead using buf.f_namemax + 1 (1531) also for exfat and vfat,
invalid length is already buf.f_namemax / NLS_MAX_CHARSET_SIZE + 1 (256).

> > +	if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC)
> > +		valid_fname = SAFE_MALLOC(buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1);
> > +	else
> > +		valid_fname = SAFE_MALLOC(buf.f_namemax - 1);

> > -		if (TEST_RETURN == -1) {
> > -			tst_resm(TFAIL | TTERRNO, "statvfs(%s, ...) failed",
> > -				 TEST_PATH);
> > -		} else {
> > -			tst_resm(TPASS, "statvfs(%s, ...) passed", TEST_PATH);
> > -		}
> > +	memset(toolong_fname, 'b', buf.f_namemax + 1);
> > +	if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC)
> > +		memset(valid_fname, 'a', buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1);
> > +	else
> > +		memset(valid_fname, 'a', buf.f_namemax - 1);
Also valid length is for buf.f_namemax, not buf.f_namemax - 1. I guess -1 is for
\0 (NULL terminator), but tests work even with just buf.f_namemax.

Also adding variable to hold the length makes source more readable.
How about this?

	struct statvfs buf;
	char *valid_fname, *toolong_fname;
	long fs_type;
	long valid_len;

	fs_type = tst_fs_type(TEST_PATH);

	TST_EXP_PASS(statvfs(TEST_PATH, &buf));

	valid_len = buf.f_namemax;
	if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC)
		valid_len = buf.f_namemax / NLS_MAX_CHARSET_SIZE;

	valid_fname = SAFE_MALLOC(valid_len);
	memset(valid_fname, 'a', valid_len);

	toolong_fname = SAFE_MALLOC(valid_len + 1);
	memset(toolong_fname, 'b', valid_len + 1);

Final diff is below.

Kind regards,
Petr

diff --git testcases/kernel/syscalls/statvfs/statvfs01.c testcases/kernel/syscalls/statvfs/statvfs01.c
index 034835da7d..f357855eb1 100644
--- testcases/kernel/syscalls/statvfs/statvfs01.c
+++ testcases/kernel/syscalls/statvfs/statvfs01.c
@@ -25,22 +25,21 @@ static void run(void)
 	struct statvfs buf;
 	char *valid_fname, *toolong_fname;
 	long fs_type;
+	long valid_len;
 
 	fs_type = tst_fs_type(TEST_PATH);
 
 	TST_EXP_PASS(statvfs(TEST_PATH, &buf));
 
-	toolong_fname = SAFE_MALLOC(buf.f_namemax + 1);
+	valid_len = buf.f_namemax;
 	if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC)
-		valid_fname = SAFE_MALLOC(buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1);
-	else
-		valid_fname = SAFE_MALLOC(buf.f_namemax - 1);
+		valid_len = buf.f_namemax / NLS_MAX_CHARSET_SIZE;
 
-	memset(toolong_fname, 'b', buf.f_namemax + 1);
-	if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC)
-		memset(valid_fname, 'a', buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1);
-	else
-		memset(valid_fname, 'a', buf.f_namemax - 1);
+	valid_fname = SAFE_MALLOC(valid_len);
+	memset(valid_fname, 'a', valid_len);
+
+	toolong_fname = SAFE_MALLOC(valid_len + 1);
+	memset(toolong_fname, 'b', valid_len + 1);
 
 	TST_EXP_FD(creat(valid_fname, 0444));
 	SAFE_CLOSE(TST_RET);

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

  reply	other threads:[~2022-12-22  9:49 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-01  8:43 [LTP] [PATCH v3] statvfs01: Convert to new LTP API Avinesh Kumar
2022-12-01 11:11 ` [LTP] [PATCH v4] " Avinesh Kumar
2022-12-05 10:38   ` Richard Palethorpe
2022-12-16 10:18     ` Petr Vorel
2022-12-20  7:57       ` [LTP] [PATCH v5] " Avinesh Kumar
2022-12-20 12:07         ` Richard Palethorpe
2022-12-22  9:49           ` Petr Vorel [this message]
2022-12-22  9:53             ` Richard Palethorpe
2022-12-23 14:40               ` Avinesh Kumar

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=Y6QoLAfvY/W175WH@pevik \
    --to=pvorel@suse.cz \
    --cc=ltp@lists.linux.it \
    --cc=rpalethorpe@suse.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox