All of lore.kernel.org
 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 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.