public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH 2/2] syscalls/fchown: Convert fchown05 to the new API
Date: Thu, 3 Jun 2021 14:05:22 +0200	[thread overview]
Message-ID: <YLjFgjJ94RuE7zRe@yuki> (raw)
In-Reply-To: <20210531112522.9082-3-xieziyao@huawei.com>

Hi!
> -	{"Change Owner/Group ids", 700, 701},
> -	{"Change Owner id only", 702, -1},
> -	{"Change Owner id only", 703, 701},
> -	{"Change Group id only", -1, 704},
> -	{"Change Group id only", 703, 705},
> -	{NULL, 0, 0}
> +	{700, 701},
> +	{702, 701},
> +	{702, 703},
> +	{704, 705}
>  };

Can we please keep the tests where we check that -1 does not change the
value and where we asert that the previously set value is still there
after the fchown call?

We may as well add a check that fchown(fd, -1, -1) is no-op.

> +	TST_EXP_PASS(FCHOWN(fd, tc[i].uid, tc[i].gid));

This produces a bit ugly output, what about adding format string and
parameters like:

	TST_EXP_PASS(FCHOWN(fd, tc[i].uid, tc[i].gid),
	             "fchwon(%i, %i, %i)", fd, tc[i].uid, tc[i].gid);

I guess I will push a similar patch that fixes this for fchown04 as well.

>  static void cleanup(void)
>  {
> -	if (fildes > 0 && close(fildes))
> -		tst_resm(TWARN | TERRNO, "close(%s) Failed", TESTFILE);
> -
> -	tst_rmdir();
> +	SAFE_CLOSE(fd);

We really have to check if the fd has been opened, since the test can
exit in any SAFE_MACRO() so this has to be:

	if (fd > 0)
		SAFE_CLOSE(fd);


Other than these this is a nice cleanup.

-- 
Cyril Hrubis
chrubis@suse.cz

  reply	other threads:[~2021-06-03 12:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-31 11:25 [LTP] [PATCH 0/2] syscalls/fchown: Convert fchown{04, 05} to the new API Xie Ziyao
2021-05-31 11:25 ` [LTP] [PATCH 1/2] syscalls/fchown: Convert fchown04 " Xie Ziyao
2021-06-03 11:51   ` Cyril Hrubis
2021-05-31 11:25 ` [LTP] [PATCH 2/2] syscalls/fchown: Convert fchown05 " Xie Ziyao
2021-06-03 12:05   ` Cyril Hrubis [this message]
2021-06-07  7:46     ` Xie Ziyao

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=YLjFgjJ94RuE7zRe@yuki \
    --to=chrubis@suse.cz \
    --cc=ltp@lists.linux.it \
    /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