xiaoxiang781216 commented on code in PR #3673: URL: https://github.com/apache/nuttx-apps/pull/3673#discussion_r3687623073
########## testing/ostest/ostest.h: ########## @@ -282,12 +282,58 @@ void priority_inheritance(void); void sched_lock_test(void); +/* The fork family **********************************************************/ + +/* Until nuttx has the three separate primitives, ARCH_HAVE_FORK stands in + * for task_fork(): today's fork() *is* task_fork(), so the test that has + * always covered that primitive keeps running under its own name. A nuttx + * without ARCH_HAVE_TASK_FORK is the pre-split one, and only there does the + * substitution apply -- once the split has landed, TASK_FORK says whether + * task_fork() was built, and nothing stands in for it. + * + * vfork_test() and fork_test() deliberately have no such mapping. They + * check semantics a pre-split nuttx does not describe -- the parent + * suspension and the private copy -- and ARCH_HAVE_VFORK is the evidence + * that the split has landed. + * + * This block comes out with the split. + */ + +#if defined(CONFIG_TASK_FORK) || \ + (defined(CONFIG_ARCH_HAVE_FORK) && !defined(CONFIG_ARCH_HAVE_TASK_FORK)) +# define OSTEST_HAVE_TASK_FORK 1 +#endif + +#ifdef CONFIG_ARCH_HAVE_VFORK +# define OSTEST_HAVE_VFORK 1 +#endif + +#if defined(CONFIG_ARCH_HAVE_FORK) && defined(CONFIG_ARCH_HAVE_VFORK) +# define OSTEST_HAVE_FORK 1 +#endif + +#if defined(OSTEST_HAVE_TASK_FORK) && !defined(CONFIG_TASK_FORK) Review Comment: why need the above mapping? it's better to just check CONFIG_ARCH_TASK_FORK/CONFIG_ARCH_FORK/CONFIG_ARCH_VFORK. ########## testing/ostest/vfork.c: ########## @@ -31,59 +31,100 @@ #include <stdbool.h> #include <stdio.h> #include <stdlib.h> +#include <sys/wait.h> #include <unistd.h> #include "ostest.h" -#if defined(CONFIG_ARCH_HAVE_FORK) && defined(CONFIG_SCHED_WAITPID) +#ifdef OSTEST_HAVE_VFORK Review Comment: remove too ########## testing/ostest/Makefile: ########## @@ -144,10 +144,21 @@ CSRCS += sigev_thread.c endif endif -ifeq ($(CONFIG_ARCH_HAVE_FORK),y) -ifeq ($(CONFIG_SCHED_WAITPID),y) +# Each test is built where the primitive it tests exists. See ostest.h for the +# one transitional exception. + +ifeq ($(CONFIG_TASK_FORK),y) +CSRCS += task_fork.c +else ifeq ($(CONFIG_ARCH_HAVE_FORK)$(CONFIG_ARCH_HAVE_TASK_FORK),y) Review Comment: remove ########## testing/ostest/ostest.h: ########## @@ -282,12 +282,58 @@ void priority_inheritance(void); void sched_lock_test(void); +/* The fork family **********************************************************/ + +/* Until nuttx has the three separate primitives, ARCH_HAVE_FORK stands in + * for task_fork(): today's fork() *is* task_fork(), so the test that has + * always covered that primitive keeps running under its own name. A nuttx + * without ARCH_HAVE_TASK_FORK is the pre-split one, and only there does the + * substitution apply -- once the split has landed, TASK_FORK says whether + * task_fork() was built, and nothing stands in for it. + * + * vfork_test() and fork_test() deliberately have no such mapping. They + * check semantics a pre-split nuttx does not describe -- the parent + * suspension and the private copy -- and ARCH_HAVE_VFORK is the evidence + * that the split has landed. + * + * This block comes out with the split. + */ + +#if defined(CONFIG_TASK_FORK) || \ + (defined(CONFIG_ARCH_HAVE_FORK) && !defined(CONFIG_ARCH_HAVE_TASK_FORK)) Review Comment: why need check CONFIG_ARCH_HAVE_FORK/CONFIG_ARCH_HAVE_TASK_FORK ########## testing/ostest/Makefile: ########## @@ -144,10 +144,21 @@ CSRCS += sigev_thread.c endif endif -ifeq ($(CONFIG_ARCH_HAVE_FORK),y) -ifeq ($(CONFIG_SCHED_WAITPID),y) +# Each test is built where the primitive it tests exists. See ostest.h for the +# one transitional exception. + +ifeq ($(CONFIG_TASK_FORK),y) +CSRCS += task_fork.c +else ifeq ($(CONFIG_ARCH_HAVE_FORK)$(CONFIG_ARCH_HAVE_TASK_FORK),y) +CSRCS += task_fork.c +endif + +ifeq ($(CONFIG_ARCH_HAVE_VFORK),y) CSRCS += vfork.c endif + +ifeq ($(CONFIG_ARCH_HAVE_FORK)$(CONFIG_ARCH_HAVE_VFORK),yy) Review Comment: why need check CONFIG_ARCH_HAVE_VFORK for fork test ########## testing/ostest/fork.c: ########## @@ -0,0 +1,280 @@ +/**************************************************************************** + * apps/testing/ostest/fork.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <assert.h> +#include <errno.h> +#include <stdbool.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/wait.h> +#include <unistd.h> + +#include "ostest.h" + +#ifdef OSTEST_HAVE_FORK Review Comment: already done in Makefile, why need check again? ########## testing/ltp/CMakeLists.txt: ########## @@ -86,6 +86,12 @@ if(CONFIG_TESTING_LTP) list(APPEND BLACKWORDS "pthread_spin_init" "pthread_spin_destroy" "pthread_spin_trylock") endif() + + # See testing/ltp/Makefile. + + if(NOT CONFIG_ARCH_HAVE_FORK AND NOT CONFIG_FORK_IS_TASK_FORK) Review Comment: why need CONFIG_FORK_IS_TASK_FORK -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
