From: "xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com>
To: "rpalethorpe@suse.de" <rpalethorpe@suse.de>,
"liaohj.jy@fujitsu.com" <liaohj.jy@fujitsu.com>
Cc: "ltp@lists.linux.it" <ltp@lists.linux.it>
Subject: Re: [LTP] [PATCH] syscalls/fork01:Convert into new api
Date: Tue, 18 Oct 2022 06:39:34 +0000 [thread overview]
Message-ID: <667dc5ef-1e19-fcb3-64cd-38eee906e98d@fujitsu.com> (raw)
In-Reply-To: <87h702oce4.fsf@suse.de>
Hi Richard
Since Liao has did other work, I will continue to do these cleanup works.
> Hello,
>
> Liao Huangjie <liaohj.jy@fujitsu.com> writes:
>
>> From: Huangjie Liao <liaohj.jy@fujitsu.com>
>>
>> Signed-off-by: Huangjie Liao <liaohj.jy@fujitsu.com>
>> ---
>> testcases/kernel/syscalls/fork/fork01.c | 253 ++++++------------------
>> 1 file changed, 55 insertions(+), 198 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/fork/fork01.c b/testcases/kernel/syscalls/fork/fork01.c
>> index 00d7c45c4..8ba712805 100644
>> --- a/testcases/kernel/syscalls/fork/fork01.c
>> +++ b/testcases/kernel/syscalls/fork/fork01.c
>> @@ -1,91 +1,17 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> /*
>> * Copyright (c) 2000 Silicon Graphics, Inc. All Rights Reserved.
>> + * Author: Kathy Olmsted
>> + * Co-Pilot: Steve Shaw
>> + */
>> +
>> +/*\
>> + *[Description]
>> *
>> - * 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.
>> - *
>> - * Further, this software is distributed without any warranty that it is
>> - * free of the rightful claim of any third person regarding infringement
>> - * or the like. Any license provided herein, whether implied or
>> - * otherwise, applies only to this software file. Patent licenses, if
>> - * any, provided herein do not apply to combinations of this program with
>> - * other software, or any other product whatsoever.
>> - *
>> - * You should have received a copy of the GNU General Public License along
>> - * with this program; if not, write the Free Software Foundation, Inc.,
>> - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>> - *
>> - * Contact information: Silicon Graphics, Inc., 1600 Amphitheatre Pkwy,
>> - * Mountain View, CA 94043, or:
>> - *
>> - * http://www.sgi.com
>> - *
>> - * For further information regarding this notice, see:
>> - *
>> - * http://oss.sgi.com/projects/GenInfo/NoticeExplan/
>> - *
>> - *
>> - * OS Test - Silicon Graphics, Inc.
>> - * TEST IDENTIFIER : fork01
>> - * EXECUTED BY : anyone
>> - * TEST TITLE : Basic test for fork(2)
>> - * PARENT DOCUMENT : frktds02
>> - * TEST CASE TOTAL : 2
>> - * WALL CLOCK TIME : 1
>> - * CPU TYPES : ALL
>> - * AUTHOR : Kathy Olmsted
>> - * CO-PILOT : Steve Shaw
>> - * DATE STARTED : 06/17/92
>> - * INITIAL RELEASE : UNICOS 7.0
>> - * TEST CASES
>> - * 1.) fork returns without error
>> - * 2.) fork returns the pid of the child
>> - * INPUT SPECIFICATIONS
>> - * The standard options for system call tests are accepted.
>> - * (See the parse_opts(3) man page).
>> - * OUTPUT SPECIFICATIONS
>> - * DURATION
>> - * Terminates - with frequency and infinite modes.
>> - * SIGNALS
>> - * Uses SIGUSR1 to pause before test if option set.
>> - * (See the parse_opts(3) man page).
>> - * RESOURCES
>> - * None
>> - * ENVIRONMENTAL NEEDS
>> - * No run-time environmental needs.
>> - * SPECIAL PROCEDURAL REQUIREMENTS
>> - * None
>> - * INTERCASE DEPENDENCIES
>> - * None
>> - * DETAILED DESCRIPTION
>> - * Setup:
>> - * Setup signal handling.
>> - * Pause for SIGUSR1 if option specified.
>> - * Test:
>> - * Loop if the proper options are given.
>> - * fork()
>> - * Check return code, if system call failed (return=-1)
>> - * Log the errno and Issue a FAIL message.
>> - * Otherwise, Issue a PASS message.
>> - * CHILD:
>> - * determine PID
>> - * write to PID to a file and close the file
>> - * exit
>> - * PARENT:
>> - * wait for child to exit
>> - * read child PID from file
>> - * compare child PID to fork() return code and report
>> - * results
>> - *
>> - * Cleanup:
>> - * Print errno log and/or timing stats if options given
>> + * This case tests fork()'s basic function.
>
> The original descrption had more info than this. In particular:
>
>> - * 1.) fork returns without error
>> - * 2.) fork returns the pid of the child
Ok, will keep it.
>
>
>> */
>>
>> +#define _GNU_SOURCE
>> #include <errno.h>
>> #include <string.h>
>> #include <signal.h>
>> @@ -93,142 +19,73 @@
>> #include <stdlib.h>
>> #include <sys/types.h>
>> #include <sys/wait.h>
>> -#include "test.h"
>> +#include <unistd.h>
>> +#include <stdio.h>
>> +#include "tst_test.h"
>>
>> #define KIDEXIT 42
>> -static void setup();
>> -static void cleanup();
>> -
>> #define LINE_SZ 20
>> #define FILENAME "childpid"
>>
>> -char *TCID = "fork01";
>> -int TST_TOTAL = 2;
>> +static char buf[LINE_SZ];
>
> This doesn't need to be a global var. In fact we don't need it at all
> (see below).
>
>>
>> -/*
>> - * child_pid - the child side of the test
>> - * determine the PID and write to a file
>> - */
>> -static void child_pid(void)
>> +static void do_child(void)
>> {
>> + int fd;
>>
>> - int fildes;
>> - char tmp_line[LINE_SZ];
>> -
>> - fildes = creat(FILENAME, 0700);
>> - sprintf(tmp_line, "%d\n", getpid());
>> - write(fildes, tmp_line, LINE_SZ);
>> - close(fildes);
>> -
>> + fd = creat(FILENAME, 0700);
>> + sprintf(buf, "%d\n", getpid());
>> + SAFE_WRITE(1, fd, buf, LINE_SZ);
>> + SAFE_CLOSE(fd);
>> }
>
> This whole function can be replaced with SAFE_FILE_PRINTF i.e
> SAFE_FILE_PRINTF(FILENAME, "%d", getpid());
>
Yes.
>>
>> -/*
>> - * parent_pid - the parent side of the test
>> - * read the value determined by the child
>> - * compare and report results
>> - */
>> -static void parent_pid(void)
>> +static void check_child_pid(int pid)
>> {
>> + int fd;
>> + pid_t child_pid;
>>
>> - int fildes;
>> - char tmp_line[LINE_SZ];
>> - pid_t child_id;
>> + fd = SAFE_OPEN(FILENAME, O_RDWR);
>> + SAFE_READ(1, fd, buf, LINE_SZ);
>
> This can be replaced with SAFE_FILE_SCANF(FILENAME, "%d", &child_pid);
Yes.
>
>>
>> - fildes = open(FILENAME, O_RDWR);
>> - if (fildes == -1) {
>> - tst_brkm(TBROK, cleanup,
>> - "parent open failed. errno: %d (%s)\n",
>> - errno, strerror(errno));
>> + child_pid = atoi(buf);
>> + if (child_pid != pid) {
>> + tst_res(TFAIL, "child reported a pid of %d. parent received %d from fork()",
>> + child_pid, pid);
>
> This can be replaced with TST_EXP_EQ_LI(child_pid, pid).
Will do.
>
>> } else {
>> - if (read(fildes, tmp_line, LINE_SZ) == 0) {
>> - tst_brkm(TBROK, cleanup,
>> - "fork(): parent failed to read PID from file errno: %d (%s)",
>> - errno, strerror(errno));
>> - } else {
>> - child_id = atoi(tmp_line);
>> - if (TEST_RETURN != child_id) {
>> - tst_resm(TFAIL,
>> - "child reported a pid of %d. parent received %ld from fork()",
>> - child_id, TEST_RETURN);
>> - } else {
>> - tst_resm(TPASS,
>> - "child pid and fork() return agree: %d",
>> - child_id);
>> - }
>> - }
>> - close(fildes);
>> - }
>> -}
>> -
>> -int main(int ac, char **av)
>> -{
>> - int lc;
>> - int fails;
>> - int kid_status, wait_status;
>> -
>> - tst_parse_opts(ac, av, NULL, NULL);
>> -
>> - setup();
>> -
>> - for (lc = 0; TEST_LOOPING(lc); lc++) {
>> - tst_count = 0;
>> - fails = 0;
>> -
>> - TEST(fork());
>> - if (TEST_RETURN == -1) {
>> - tst_resm(TFAIL, "fork() Failed, errno=%d : %s",
>> - TEST_ERRNO, strerror(TEST_ERRNO));
>> - tst_resm(TBROK, "unable to continue");
>> - }
>> - if (TEST_RETURN == 0) {
>> - /* child */
>> - child_pid();
>> - exit(KIDEXIT);
>> - } else {
>> - /* parent */
>> - tst_resm(TPASS, "fork() returned %ld",
>> - TEST_RETURN);
>> - /* wait for the child to complete */
>> - wait_status = waitpid(TEST_RETURN, &kid_status, 0);
>> -
>> - if (wait_status == TEST_RETURN) {
>> - if (kid_status != KIDEXIT << 8) {
>> - tst_resm(TBROK,
>> - "incorrect child status returned on wait(): %d",
>> - kid_status);
>> - fails++;
>> - }
>> - } else {
>> - tst_resm(TBROK,
>> - "wait() for child status failed with %d errno: %d : %s",
>> - wait_status, errno,
>> - strerror(errno));
>> - fails++;
>> - }
>> - if (fails == 0) {
>> - /* verification tests */
>> - parent_pid();
>> - }
>> - } /* TEST_RETURN */
>> + tst_res(TPASS, "child pid and fork() return agree: %d", pid);
>> }
>>
>> - cleanup();
>> - tst_exit();
>> + SAFE_CLOSE(fd);
>> }
>>
>> -static void setup(void)
>> +static void verify_fork(void)
>> {
>> + int kid_status, term_pid, pid;
>>
>> - tst_sig(FORK, DEF_HANDLER, cleanup);
>> -
>> - TEST_PAUSE;
>> -
>> - tst_tmpdir();
>> -}
>> + pid = SAFE_FORK();
>> + if (!pid) {
>> + do_child();
>> + exit(KIDEXIT);
>> + }
>>
>> -static void cleanup(void)
>> -{
>> + term_pid = SAFE_WAITPID(pid, &kid_status, 0);
>> + if (term_pid == pid) {
>> + if (kid_status != KIDEXIT << 8)
>> + tst_res(TFAIL, "incorrect child status returned %d", kid_status);
>> + else
>> + tst_res(TPASS, "correct child status returned
>> %d", kid_status);
>
> The original test is not strictly correct. We should check if the child
> exited with WIFEXITED and use the WEXITSTATUS macro to get the exit
> status.
OK.
>
>>
>> - tst_rmdir();
>> + check_child_pid(pid);
>> + } else {
>> + tst_res(TFAIL, "waitpid() returns %d instead of expected pid %d", term_pid, pid);
>> + }
>>
>> + tst_reap_children();
>> }
>> +
>> +static struct tst_test test = {
>> + .needs_tmpdir = 1,
>> + .needs_root = 1,
>
> It doesn't need root.
Yes.
Best Regards
Yang Xu
>
>> + .forks_child = 1,
>> + .test_all = verify_fork,
>> +};
>> --
>> 2.27.0
>
>
--
Mailing list info: https://lists.linux.it/listinfo/ltp
prev parent reply other threads:[~2022-10-18 6:39 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-15 1:21 [LTP] [PATCH] syscalls/fork01:Convert into new api Liao Huangjie
2022-10-17 12:08 ` Richard Palethorpe
2022-10-18 6:39 ` xuyang2018.jy [this message]
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=667dc5ef-1e19-fcb3-64cd-38eee906e98d@fujitsu.com \
--to=xuyang2018.jy@fujitsu.com \
--cc=liaohj.jy@fujitsu.com \
--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 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.