All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 1/2] fchownat/fchownat01: Convert to new API
@ 2021-11-17  3:39 tangmeng
  2021-11-17  3:39 ` [LTP] [PATCH 2/2] fchownat/fchownat02: " tangmeng
  2021-12-08 16:06 ` [LTP] [PATCH 1/2] fchownat/fchownat01: " Cyril Hrubis
  0 siblings, 2 replies; 4+ messages in thread
From: tangmeng @ 2021-11-17  3:39 UTC (permalink / raw)
  To: ltp; +Cc: tangmeng

Signed-off-by: tangmeng <tangmeng@uniontech.com>
---
 testcases/kernel/syscalls/fchownat/fchownat.h |  15 +-
 .../kernel/syscalls/fchownat/fchownat01.c     | 131 +++++-------------
 2 files changed, 39 insertions(+), 107 deletions(-)

diff --git a/testcases/kernel/syscalls/fchownat/fchownat.h b/testcases/kernel/syscalls/fchownat/fchownat.h
index a95c26f93..75c435dab 100644
--- a/testcases/kernel/syscalls/fchownat/fchownat.h
+++ b/testcases/kernel/syscalls/fchownat/fchownat.h
@@ -1,20 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2014 Fujitsu Ltd.
  *
- * This program is free software;  you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Library General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
- *
  */

 #ifndef FCHOWNAT_H
diff --git a/testcases/kernel/syscalls/fchownat/fchownat01.c b/testcases/kernel/syscalls/fchownat/fchownat01.c
index 9f4ecded7..b18af54f4 100644
--- a/testcases/kernel/syscalls/fchownat/fchownat01.c
+++ b/testcases/kernel/syscalls/fchownat/fchownat01.c
@@ -1,60 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  *   Copyright (c) International Business Machines  Corp., 2006
  *   AUTHOR: Yi Yang <yyangcdl@cn.ibm.com>
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software Foundation,
- *   Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
-/*
- * DESCRIPTION
- *	This test case will verify basic function of fchownat
- *	added by kernel 2.6.16 or up.
+
+/*\
+ * [Description]
+ *
+ * This test case will verify basic function of fchownat
+ * added by kernel 2.6.16 or up.
  */

 #define _GNU_SOURCE

-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <unistd.h>
-#include <stdlib.h>
-#include <errno.h>
-#include <string.h>
-#include <signal.h>
-
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
 #include "fchownat.h"
 #include "lapi/fcntl.h"

 #define TESTFILE	"testfile"

-static void setup(void);
-static void cleanup(void);
-
 static int dir_fd;
 static int fd;
 static int no_fd = -1;
 static int cu_fd = AT_FDCWD;

-static struct test_case_t {
+static struct tcase {
 	int exp_ret;
 	int exp_errno;
 	int flag;
 	int *fds;
 	char *filenames;
-} test_cases[] = {
+} tcases[] = {
 	{0, 0, 0, &dir_fd, TESTFILE},
 	{-1, ENOTDIR, 0, &fd, TESTFILE},
 	{-1, EBADF, 0, &no_fd, TESTFILE},
@@ -62,77 +38,46 @@ static struct test_case_t {
 	{0, 0, 0, &cu_fd, TESTFILE},
 };

-char *TCID = "fchownat01";
-int TST_TOTAL = ARRAY_SIZE(test_cases);
-static void fchownat_verify(const struct test_case_t *);
-
-int main(int ac, char **av)
+static void verify_fchownat(unsigned int i)
 {
-	int lc;
-	int i;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
+	struct tcase *tc = &tcases[i];

-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-		for (i = 0; i < TST_TOTAL; i++)
-			fchownat_verify(&test_cases[i]);
+	if (tc->exp_ret == 0) {
+		TST_EXP_PASS(fchownat(*(tc->fds), tc->filenames, geteuid(),
+			getegid(), tc->flag),
+		 	"fchownat() returned %ld, expected %d, errno=%d",
+			 TST_RET, tc->exp_ret, tc->exp_errno);
+	} else {
+		TST_EXP_FAIL(fchownat(*(tc->fds), tc->filenames, geteuid(),
+			getegid(), tc->flag), tc->exp_errno,
+			 "fchownat() expected errno %d: %s",
+			 tc->exp_ret, strerror(tc->exp_errno));
 	}
-
-	cleanup();
-	tst_exit();
 }

 static void setup(void)
 {
-	if ((tst_kvercmp(2, 6, 16)) < 0)
-		tst_brkm(TCONF, NULL, "This test needs kernel 2.6.16 or newer");
-
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
-
-	tst_tmpdir();
-
-	dir_fd = SAFE_OPEN(cleanup, "./", O_DIRECTORY);
-
-	SAFE_TOUCH(cleanup, TESTFILE, 0600, NULL);
-
-	fd = SAFE_OPEN(cleanup, "testfile2", O_CREAT | O_RDWR, 0600);
-}
-
-static void fchownat_verify(const struct test_case_t *test)
-{
-	TEST(fchownat(*(test->fds), test->filenames, geteuid(),
-		      getegid(), test->flag));
+	dir_fd = SAFE_OPEN("./", O_DIRECTORY);

-	if (TEST_RETURN != test->exp_ret) {
-		tst_resm(TFAIL | TTERRNO,
-			 "fchownat() returned %ld, expected %d, errno=%d",
-			 TEST_RETURN, test->exp_ret, test->exp_errno);
-		return;
-	}
+	SAFE_TOUCH(TESTFILE, 0600, NULL);

-	if (TEST_ERRNO == test->exp_errno) {
-		tst_resm(TPASS | TTERRNO,
-			 "fchownat() returned the expected errno %d: %s",
-			 test->exp_ret, strerror(test->exp_errno));
-	} else {
-		tst_resm(TFAIL | TTERRNO,
-			 "fchownat() failed unexpectedly; expected: %d - %s",
-			 test->exp_errno, strerror(test->exp_errno));
-	}
+	fd = SAFE_OPEN("testfile2", O_CREAT | O_RDWR, 0600);
 }

 static void cleanup(void)
 {
-	if (fd > 0)
-		close(fd);
+       if (fd > 0)
+               close(fd);

-	if (dir_fd > 0)
-		close(dir_fd);
-
-	tst_rmdir();
+       if (dir_fd > 0)
+               close(dir_fd);
 }
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.min_kver = "2.6.16",
+	.test = verify_fchownat,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_tmpdir = 1,
+};
--
2.20.1




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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [LTP] [PATCH 2/2] fchownat/fchownat02: Convert to new API
  2021-11-17  3:39 [LTP] [PATCH 1/2] fchownat/fchownat01: Convert to new API tangmeng
@ 2021-11-17  3:39 ` tangmeng
  2021-12-08 16:18   ` Cyril Hrubis
  2021-12-08 16:06 ` [LTP] [PATCH 1/2] fchownat/fchownat01: " Cyril Hrubis
  1 sibling, 1 reply; 4+ messages in thread
From: tangmeng @ 2021-11-17  3:39 UTC (permalink / raw)
  To: ltp; +Cc: tangmeng

Signed-off-by: tangmeng <tangmeng@uniontech.com>
---
 .../kernel/syscalls/fchownat/fchownat02.c     | 121 +++++-------------
 1 file changed, 34 insertions(+), 87 deletions(-)

diff --git a/testcases/kernel/syscalls/fchownat/fchownat02.c b/testcases/kernel/syscalls/fchownat/fchownat02.c
index d19f3f3b8..116d4ca37 100644
--- a/testcases/kernel/syscalls/fchownat/fchownat02.c
+++ b/testcases/kernel/syscalls/fchownat/fchownat02.c
@@ -1,132 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) 2014 Fujitsu Ltd.
  * Author: Zeng Linggang <zenglg.jy@cn.fujitsu.com>
  *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program.
  */
-/*
- * Test Description:
- *   Verify that,
- *   The flag of fchownat() is AT_SYMLINK_NOFOLLOW and the pathname would
- *   not be dereferenced if the pathname is a symbolic link.
+
+/*\
+ * [Description]
+ *
+ * Verify that,
+ * The flag of fchownat() is AT_SYMLINK_NOFOLLOW and the pathname would
+ * not be dereferenced if the pathname is a symbolic link.
  */

 #define _GNU_SOURCE

-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <unistd.h>
-#include <stdlib.h>
-#include <errno.h>
-#include <string.h>
-#include <signal.h>
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
 #include "fchownat.h"
 #include "lapi/fcntl.h"

 #define TESTFILE	"testfile"
 #define TESTFILE_LINK	"testfile_link"

-char *TCID = "fchownat02";
-int TST_TOTAL = 1;
-
 static int dir_fd;
 static uid_t set_uid = 1000;
 static gid_t set_gid = 1000;
-static void setup(void);
-static void cleanup(void);
-static void test_verify(void);
-static void fchownat_verify(void);
-
-int main(int ac, char **av)
-{
-	int lc;
-	int i;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-		for (i = 0; i < TST_TOTAL; i++)
-			fchownat_verify();
-	}
-
-	cleanup();
-	tst_exit();
-}

 static void setup(void)
 {
 	struct stat c_buf, l_buf;

-	if ((tst_kvercmp(2, 6, 16)) < 0)
-		tst_brkm(TCONF, NULL, "This test needs kernel 2.6.16 or newer");
-
-	tst_require_root();
+	dir_fd = SAFE_OPEN("./", O_DIRECTORY);

-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+	SAFE_TOUCH(TESTFILE, 0600, NULL);

-	TEST_PAUSE;
+	SAFE_SYMLINK(TESTFILE, TESTFILE_LINK);

-	tst_tmpdir();
+	SAFE_STAT(TESTFILE_LINK, &c_buf);

-	dir_fd = SAFE_OPEN(cleanup, "./", O_DIRECTORY);
-
-	SAFE_TOUCH(cleanup, TESTFILE, 0600, NULL);
-
-	SAFE_SYMLINK(cleanup, TESTFILE, TESTFILE_LINK);
-
-	SAFE_STAT(cleanup, TESTFILE_LINK, &c_buf);
-
-	SAFE_LSTAT(cleanup, TESTFILE_LINK, &l_buf);
+	SAFE_LSTAT(TESTFILE_LINK, &l_buf);

 	if (l_buf.st_uid == set_uid || l_buf.st_gid == set_gid) {
-		tst_brkm(TBROK | TERRNO, cleanup,
+		tst_brk(TBROK | TERRNO,
 			 "link_uid(%d) == set_uid(%d) or link_gid(%d) == "
 			 "set_gid(%d)", l_buf.st_uid, set_uid, l_buf.st_gid,
 			 set_gid);
 	}
 }

-static void fchownat_verify(void)
-{
-	TEST(fchownat(dir_fd, TESTFILE_LINK, set_uid, set_gid,
-		      AT_SYMLINK_NOFOLLOW));
-
-	if (TEST_RETURN != 0) {
-		tst_resm(TFAIL | TTERRNO, "fchownat() failed, errno=%d : %s",
-			 TEST_ERRNO, strerror(TEST_ERRNO));
-	} else {
-		test_verify();
-	}
-}
-
-static void test_verify(void)
+static void verify_fchownat(void)
 {
 	struct stat c_buf, l_buf;

-	SAFE_STAT(cleanup, TESTFILE_LINK, &c_buf);
+	TST_EXP_PASS_SILENT(fchownat(dir_fd, TESTFILE_LINK, set_uid, set_gid,
+		      AT_SYMLINK_NOFOLLOW));
+
+	SAFE_STAT(TESTFILE_LINK, &c_buf);

-	SAFE_LSTAT(cleanup, TESTFILE_LINK, &l_buf);
+	SAFE_LSTAT(TESTFILE_LINK, &l_buf);

 	if (c_buf.st_uid != set_uid && l_buf.st_uid == set_uid &&
 	    c_buf.st_gid != set_gid && l_buf.st_gid == set_gid) {
-		tst_resm(TPASS, "fchownat() test AT_SYMLINK_NOFOLLOW success");
+		tst_res(TPASS, "fchownat() test AT_SYMLINK_NOFOLLOW success");
 	} else {
-		tst_resm(TFAIL,
+		tst_res(TFAIL,
 			 "fchownat() test AT_SYMLINK_NOFOLLOW fail with uid=%d "
 			 "link_uid=%d set_uid=%d | gid=%d link_gid=%d "
 			 "set_gid=%d", c_buf.st_uid, l_buf.st_uid, set_uid,
@@ -136,5 +73,15 @@ static void test_verify(void)

 static void cleanup(void)
 {
-	tst_rmdir();
+       if (dir_fd > 0)
+               close(dir_fd);
 }
+
+static struct tst_test test = {
+        .min_kver = "2.6.16",
+        .test_all = verify_fchownat,
+        .setup = setup,
+        .cleanup = cleanup,
+        .needs_tmpdir = 1,
+	.needs_root = 1,
+};
--
2.20.1




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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH 1/2] fchownat/fchownat01: Convert to new API
  2021-11-17  3:39 [LTP] [PATCH 1/2] fchownat/fchownat01: Convert to new API tangmeng
  2021-11-17  3:39 ` [LTP] [PATCH 2/2] fchownat/fchownat02: " tangmeng
@ 2021-12-08 16:06 ` Cyril Hrubis
  1 sibling, 0 replies; 4+ messages in thread
From: Cyril Hrubis @ 2021-12-08 16:06 UTC (permalink / raw)
  To: tangmeng; +Cc: ltp

Hi!
First of all the patch has whitespace issues, please make sure to run
make check-fchownat01 and fix all the warnings before commiting the
changes.

> ---
>  testcases/kernel/syscalls/fchownat/fchownat.h |  15 +-
>  .../kernel/syscalls/fchownat/fchownat01.c     | 131 +++++-------------
>  2 files changed, 39 insertions(+), 107 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fchownat/fchownat.h b/testcases/kernel/syscalls/fchownat/fchownat.h
> index a95c26f93..75c435dab 100644
> --- a/testcases/kernel/syscalls/fchownat/fchownat.h
> +++ b/testcases/kernel/syscalls/fchownat/fchownat.h
> @@ -1,20 +1,7 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * Copyright (c) 2014 Fujitsu Ltd.
>   *
> - * This program is free software;  you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU Library General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> - *
>   */

This header has to be fixed as well, the new API uses tst_syscall()
instead of ltp_syscall().

>  #ifndef FCHOWNAT_H
> diff --git a/testcases/kernel/syscalls/fchownat/fchownat01.c b/testcases/kernel/syscalls/fchownat/fchownat01.c
> index 9f4ecded7..b18af54f4 100644
> --- a/testcases/kernel/syscalls/fchownat/fchownat01.c
> +++ b/testcases/kernel/syscalls/fchownat/fchownat01.c
> @@ -1,60 +1,36 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   *   Copyright (c) International Business Machines  Corp., 2006
>   *   AUTHOR: Yi Yang <yyangcdl@cn.ibm.com>
> - *
> - *   This program is free software;  you can redistribute it and/or modify
> - *   it under the terms of the GNU General Public License as published by
> - *   the Free Software Foundation; either version 2 of the License, or
> - *   (at your option) any later version.
> - *
> - *   This program is distributed in the hope that it will be useful,
> - *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
> - *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> - *   the GNU General Public License for more details.
> - *
> - *   You should have received a copy of the GNU General Public License
> - *   along with this program;  if not, write to the Free Software Foundation,
> - *   Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */
> -/*
> - * DESCRIPTION
> - *	This test case will verify basic function of fchownat
> - *	added by kernel 2.6.16 or up.
> +
> +/*\
> + * [Description]
> + *
> + * This test case will verify basic function of fchownat
> + * added by kernel 2.6.16 or up.
>   */
> 
>  #define _GNU_SOURCE
> 
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <fcntl.h>
> -#include <unistd.h>
> -#include <stdlib.h>
> -#include <errno.h>
> -#include <string.h>
> -#include <signal.h>
> -
> -#include "test.h"
> -#include "safe_macros.h"
> +#include "tst_test.h"
>  #include "fchownat.h"
>  #include "lapi/fcntl.h"
> 
>  #define TESTFILE	"testfile"
> 
> -static void setup(void);
> -static void cleanup(void);
> -
>  static int dir_fd;
>  static int fd;
>  static int no_fd = -1;
>  static int cu_fd = AT_FDCWD;
> 
> -static struct test_case_t {
> +static struct tcase {
>  	int exp_ret;
>  	int exp_errno;
>  	int flag;
>  	int *fds;
>  	char *filenames;

These two should really be just fd and filename, since they are single
quantities they shouldn't be plural.

> -} test_cases[] = {
> +} tcases[] = {
>  	{0, 0, 0, &dir_fd, TESTFILE},
>  	{-1, ENOTDIR, 0, &fd, TESTFILE},
>  	{-1, EBADF, 0, &no_fd, TESTFILE},
> @@ -62,77 +38,46 @@ static struct test_case_t {
>  	{0, 0, 0, &cu_fd, TESTFILE},
>  };
> 
> -char *TCID = "fchownat01";
> -int TST_TOTAL = ARRAY_SIZE(test_cases);
> -static void fchownat_verify(const struct test_case_t *);
> -
> -int main(int ac, char **av)
> +static void verify_fchownat(unsigned int i)
>  {
> -	int lc;
> -	int i;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> +	struct tcase *tc = &tcases[i];
> 
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		tst_count = 0;
> -		for (i = 0; i < TST_TOTAL; i++)
> -			fchownat_verify(&test_cases[i]);
> +	if (tc->exp_ret == 0) {
> +		TST_EXP_PASS(fchownat(*(tc->fds), tc->filenames, geteuid(),
> +			getegid(), tc->flag),
> +		 	"fchownat() returned %ld, expected %d, errno=%d",
> +			 TST_RET, tc->exp_ret, tc->exp_errno);
                          ^
			  Here we should describe _what_ is being done,
			  the pass/fail messages are generated by the
			  macro itself. Printing return value or errno
			  here is wrong.

It should really be something as:

		"fchownat(%i, '%s', ..., %i)", *(tc->fds), tc->filename, tc->flag


> +	} else {
> +		TST_EXP_FAIL(fchownat(*(tc->fds), tc->filenames, geteuid(),
> +			getegid(), tc->flag), tc->exp_errno,
> +			 "fchownat() expected errno %d: %s",
> +			 tc->exp_ret, strerror(tc->exp_errno));
                          ^
			  Here as well.
>  	}
> -
> -	cleanup();
> -	tst_exit();
>  }
> 
>  static void setup(void)
>  {
> -	if ((tst_kvercmp(2, 6, 16)) < 0)
> -		tst_brkm(TCONF, NULL, "This test needs kernel 2.6.16 or newer");
> -
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> -	TEST_PAUSE;
> -
> -	tst_tmpdir();
> -
> -	dir_fd = SAFE_OPEN(cleanup, "./", O_DIRECTORY);
> -
> -	SAFE_TOUCH(cleanup, TESTFILE, 0600, NULL);
> -
> -	fd = SAFE_OPEN(cleanup, "testfile2", O_CREAT | O_RDWR, 0600);
> -}
> -
> -static void fchownat_verify(const struct test_case_t *test)
> -{
> -	TEST(fchownat(*(test->fds), test->filenames, geteuid(),
> -		      getegid(), test->flag));
> +	dir_fd = SAFE_OPEN("./", O_DIRECTORY);
> 
> -	if (TEST_RETURN != test->exp_ret) {
> -		tst_resm(TFAIL | TTERRNO,
> -			 "fchownat() returned %ld, expected %d, errno=%d",
> -			 TEST_RETURN, test->exp_ret, test->exp_errno);
> -		return;
> -	}
> +	SAFE_TOUCH(TESTFILE, 0600, NULL);
> 
> -	if (TEST_ERRNO == test->exp_errno) {
> -		tst_resm(TPASS | TTERRNO,
> -			 "fchownat() returned the expected errno %d: %s",
> -			 test->exp_ret, strerror(test->exp_errno));
> -	} else {
> -		tst_resm(TFAIL | TTERRNO,
> -			 "fchownat() failed unexpectedly; expected: %d - %s",
> -			 test->exp_errno, strerror(test->exp_errno));
> -	}
> +	fd = SAFE_OPEN("testfile2", O_CREAT | O_RDWR, 0600);
>  }
> 
>  static void cleanup(void)
>  {
> -	if (fd > 0)
> -		close(fd);
> +       if (fd > 0)
> +               close(fd);
> 
> -	if (dir_fd > 0)
> -		close(dir_fd);
> -
> -	tst_rmdir();
> +       if (dir_fd > 0)
> +               close(dir_fd);

These two should use SAFE_CLOSE()

>  }
> +
> +static struct tst_test test = {
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.min_kver = "2.6.16",
> +	.test = verify_fchownat,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_tmpdir = 1,
> +};
> --
> 2.20.1
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH 2/2] fchownat/fchownat02: Convert to new API
  2021-11-17  3:39 ` [LTP] [PATCH 2/2] fchownat/fchownat02: " tangmeng
@ 2021-12-08 16:18   ` Cyril Hrubis
  0 siblings, 0 replies; 4+ messages in thread
From: Cyril Hrubis @ 2021-12-08 16:18 UTC (permalink / raw)
  To: tangmeng; +Cc: ltp

Hi!
Same comment apply here as well, 'make check' produces many warnings
that should have been fixed.

> +/*\
> + * [Description]
> + *
> + * Verify that,
> + * The flag of fchownat() is AT_SYMLINK_NOFOLLOW and the pathname would
> + * not be dereferenced if the pathname is a symbolic link.

This is a bit broken, at least the sentence should continue with
lowercase The on the second line as continuation of the sentence that
has started with 'Verify that,'.

But it would be better rewritten to something as:

Verify that fchownat() with AT_SYMLINK_NOFOLLOW does not dereference
symbolic links.

>   */
> 
>  #define _GNU_SOURCE
> 
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <fcntl.h>
> -#include <unistd.h>
> -#include <stdlib.h>
> -#include <errno.h>
> -#include <string.h>
> -#include <signal.h>
> -#include "test.h"
> -#include "safe_macros.h"
> +#include "tst_test.h"
>  #include "fchownat.h"
>  #include "lapi/fcntl.h"
> 
>  #define TESTFILE	"testfile"
>  #define TESTFILE_LINK	"testfile_link"
> 
> -char *TCID = "fchownat02";
> -int TST_TOTAL = 1;
> -
>  static int dir_fd;
>  static uid_t set_uid = 1000;
>  static gid_t set_gid = 1000;
> -static void setup(void);
> -static void cleanup(void);
> -static void test_verify(void);
> -static void fchownat_verify(void);
> -
> -int main(int ac, char **av)
> -{
> -	int lc;
> -	int i;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		tst_count = 0;
> -		for (i = 0; i < TST_TOTAL; i++)
> -			fchownat_verify();
> -	}
> -
> -	cleanup();
> -	tst_exit();
> -}
> 
>  static void setup(void)
>  {
>  	struct stat c_buf, l_buf;
> 
> -	if ((tst_kvercmp(2, 6, 16)) < 0)
> -		tst_brkm(TCONF, NULL, "This test needs kernel 2.6.16 or newer");
> -
> -	tst_require_root();
> +	dir_fd = SAFE_OPEN("./", O_DIRECTORY);
> 
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> +	SAFE_TOUCH(TESTFILE, 0600, NULL);
> 
> -	TEST_PAUSE;
> +	SAFE_SYMLINK(TESTFILE, TESTFILE_LINK);
> 
> -	tst_tmpdir();
> +	SAFE_STAT(TESTFILE_LINK, &c_buf);
> 
> -	dir_fd = SAFE_OPEN(cleanup, "./", O_DIRECTORY);
> -
> -	SAFE_TOUCH(cleanup, TESTFILE, 0600, NULL);
> -
> -	SAFE_SYMLINK(cleanup, TESTFILE, TESTFILE_LINK);
> -
> -	SAFE_STAT(cleanup, TESTFILE_LINK, &c_buf);
> -
> -	SAFE_LSTAT(cleanup, TESTFILE_LINK, &l_buf);
> +	SAFE_LSTAT(TESTFILE_LINK, &l_buf);
> 
>  	if (l_buf.st_uid == set_uid || l_buf.st_gid == set_gid) {
> -		tst_brkm(TBROK | TERRNO, cleanup,
> +		tst_brk(TBROK | TERRNO,
>  			 "link_uid(%d) == set_uid(%d) or link_gid(%d) == "
>  			 "set_gid(%d)", l_buf.st_uid, set_uid, l_buf.st_gid,
>  			 set_gid);
>  	}

I guess that it would be easier to set the set_uid and set_gid to
l_buf.st_uid + 1 and l_buf.st_gid + 1 that way we will be sure that it's
different.

>  }
> 
> -static void fchownat_verify(void)
> -{
> -	TEST(fchownat(dir_fd, TESTFILE_LINK, set_uid, set_gid,
> -		      AT_SYMLINK_NOFOLLOW));
> -
> -	if (TEST_RETURN != 0) {
> -		tst_resm(TFAIL | TTERRNO, "fchownat() failed, errno=%d : %s",
> -			 TEST_ERRNO, strerror(TEST_ERRNO));
> -	} else {
> -		test_verify();
> -	}
> -}
> -
> -static void test_verify(void)
> +static void verify_fchownat(void)
>  {
>  	struct stat c_buf, l_buf;
> 
> -	SAFE_STAT(cleanup, TESTFILE_LINK, &c_buf);
> +	TST_EXP_PASS_SILENT(fchownat(dir_fd, TESTFILE_LINK, set_uid, set_gid,
> +		      AT_SYMLINK_NOFOLLOW));
> +
> +	SAFE_STAT(TESTFILE_LINK, &c_buf);
> 
> -	SAFE_LSTAT(cleanup, TESTFILE_LINK, &l_buf);
> +	SAFE_LSTAT(TESTFILE_LINK, &l_buf);
> 
>  	if (c_buf.st_uid != set_uid && l_buf.st_uid == set_uid &&
>  	    c_buf.st_gid != set_gid && l_buf.st_gid == set_gid) {
> -		tst_resm(TPASS, "fchownat() test AT_SYMLINK_NOFOLLOW success");
> +		tst_res(TPASS, "fchownat() test AT_SYMLINK_NOFOLLOW success");
>  	} else {
> -		tst_resm(TFAIL,
> +		tst_res(TFAIL,
>  			 "fchownat() test AT_SYMLINK_NOFOLLOW fail with uid=%d "
>  			 "link_uid=%d set_uid=%d | gid=%d link_gid=%d "
>  			 "set_gid=%d", c_buf.st_uid, l_buf.st_uid, set_uid,
> @@ -136,5 +73,15 @@ static void test_verify(void)
> 
>  static void cleanup(void)
>  {
> -	tst_rmdir();
> +       if (dir_fd > 0)
> +               close(dir_fd);

Here SAFE_CLOSE() as well.

>  }
> +
> +static struct tst_test test = {
> +        .min_kver = "2.6.16",
> +        .test_all = verify_fchownat,
> +        .setup = setup,
> +        .cleanup = cleanup,
> +        .needs_tmpdir = 1,
> +	.needs_root = 1,
> +};
> --
> 2.20.1
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-12-08 16:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-17  3:39 [LTP] [PATCH 1/2] fchownat/fchownat01: Convert to new API tangmeng
2021-11-17  3:39 ` [LTP] [PATCH 2/2] fchownat/fchownat02: " tangmeng
2021-12-08 16:18   ` Cyril Hrubis
2021-12-08 16:06 ` [LTP] [PATCH 1/2] fchownat/fchownat01: " Cyril Hrubis

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.