xiaoxiang781216 commented on code in PR #3673:
URL: https://github.com/apache/nuttx-apps/pull/3673#discussion_r3695695866


##########
interpreters/python/Makefile:
##########
@@ -114,6 +114,11 @@ ifeq ($(CONFIG_ARCH_HAVE_FORK),y)
 else
        @echo "export ac_cv_func_fork=\"no\"" >> $@
 endif
+ifneq ($(CONFIG_ARCH_HAVE_VFORK)$(CONFIG_ARCH_HAVE_FORK),)

Review Comment:
   ```suggestion
   ifneq ($(CONFIG_ARCH_HAVE_VFORK),)
   ```



##########
netutils/libwebsockets/lws_config_private.h:
##########
@@ -111,10 +114,14 @@
 /* #undef LWS_HAVE_VFORK_H */
 
 /* Define to 1 if `fork' works. */
+#ifdef CONFIG_ARCH_HAVE_FORK
 #define LWS_HAVE_WORKING_FORK
+#endif
 
 /* Define to 1 if `vfork' works. */
+#if defined(CONFIG_ARCH_HAVE_VFORK) || defined(CONFIG_ARCH_HAVE_FORK)

Review Comment:
   ```suggestion
   #ifdef CONFIG_ARCH_HAVE_VFORK
   ```



##########
testing/ltp/Makefile:
##########
@@ -44,6 +44,13 @@ BLACKWORDS  += "pthread_spin_destroy"
 BLACKWORDS  += "pthread_spin_trylock"
 endif
 
+# Where NuttX does not declare fork(), a test that calls it cannot be built.
+# The pattern spares vfork() and task_fork(), which remain available.
+
+ifeq ($(CONFIG_ARCH_HAVE_FORK)$(CONFIG_FORK_IS_TASK_FORK),)

Review Comment:
   ditto



##########
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

Review Comment:
   remove OSTEST_HAVE_XXX, check CONFIG_ARCH_HAVE_XXX directly



##########
testing/ostest/CMakeLists.txt:
##########
@@ -143,10 +143,20 @@ if(CONFIG_TESTING_OSTEST)
     endif()
   endif()
 
-  if(CONFIG_ARCH_HAVE_FORK)
-    if(CONFIG_SCHED_WAITPID)
-      list(APPEND SRCS vfork.c)
-    endif()
+  # See testing/ostest/ostest.h for the one transitional exception.
+
+  if(CONFIG_TASK_FORK)
+    list(APPEND SRCS task_fork.c)
+  elseif(CONFIG_ARCH_HAVE_FORK AND NOT CONFIG_ARCH_HAVE_TASK_FORK)
+    list(APPEND SRCS task_fork.c)
+  endif()
+
+  if(CONFIG_ARCH_HAVE_VFORK)
+    list(APPEND SRCS vfork.c)
+  endif()
+
+  if(CONFIG_ARCH_HAVE_FORK AND CONFIG_ARCH_HAVE_VFORK)

Review Comment:
   ```suggestion
     if(CONFIG_ARCH_HAVE_FORK)
   ```



##########
testing/drivers/nand_sim/Kconfig:
##########
@@ -6,6 +6,7 @@
 config TESTING_NAND_SIM
        boolean "NAND Flash Simulator"
        depends on MTD_NAND_RAM && ENABLE_ALL_SIGNALS
+       depends on TASK_FORK || (ARCH_HAVE_FORK && !ARCH_HAVE_TASK_FORK)

Review Comment:
   ```suggestion
        depends on TASK_FORK || ARCH_HAVE_FORK
   ```



##########
testing/fs/fdsantest/fdsantest_simple.c:
##########
@@ -96,6 +96,7 @@ static void test_case_overflow(void **state)
   assert_int_equal(open_count, close_count);
 }
 
+#if defined(CONFIG_ARCH_HAVE_VFORK) || defined(CONFIG_ARCH_HAVE_FORK)

Review Comment:
   ```suggestion
   #ifdef CONFIG_ARCH_HAVE_VFORK
   ```



##########
testing/ostest/CMakeLists.txt:
##########
@@ -143,10 +143,20 @@ if(CONFIG_TESTING_OSTEST)
     endif()
   endif()
 
-  if(CONFIG_ARCH_HAVE_FORK)
-    if(CONFIG_SCHED_WAITPID)
-      list(APPEND SRCS vfork.c)
-    endif()
+  # See testing/ostest/ostest.h for the one transitional exception.
+
+  if(CONFIG_TASK_FORK)
+    list(APPEND SRCS task_fork.c)
+  elseif(CONFIG_ARCH_HAVE_FORK AND NOT CONFIG_ARCH_HAVE_TASK_FORK)

Review Comment:
   remove the fallback path



##########
testing/ostest/Makefile:
##########
@@ -144,10 +144,27 @@ CSRCS += sigev_thread.c
 endif
 endif
 
+# 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_TASK_FORK),)
+# A nuttx with no ARCH_HAVE_TASK_FORK at all is the pre-split one, where
+# fork() is task_fork().  This branch comes out with the split.
 ifeq ($(CONFIG_ARCH_HAVE_FORK),y)
-ifeq ($(CONFIG_SCHED_WAITPID),y)
+CSRCS += task_fork.c
+endif
+endif
+
+ifeq ($(CONFIG_ARCH_HAVE_VFORK),y)
 CSRCS += vfork.c
 endif
+
+ifeq ($(CONFIG_ARCH_HAVE_FORK),y)
+ifeq ($(CONFIG_ARCH_HAVE_VFORK),y)

Review Comment:
   ditto



##########
testing/fs/fdsantest/fdsantest_simple.c:
##########
@@ -129,7 +131,9 @@ int main(int argc, FAR char *argv[])
     cmocka_unit_test(test_case_unowned_tagged_close),
     cmocka_unit_test(test_case_owned_tagged_close),
     cmocka_unit_test(test_case_overflow),
+#if defined(CONFIG_ARCH_HAVE_VFORK) || defined(CONFIG_ARCH_HAVE_FORK)

Review Comment:
   ```suggestion
   #ifdef CONFIG_ARCH_HAVE_VFORK
   ```



##########
testing/ostest/Makefile:
##########
@@ -144,10 +144,27 @@ CSRCS += sigev_thread.c
 endif
 endif
 
+# 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_TASK_FORK),)

Review Comment:
   ditto



##########
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)
+#  define task_fork() fork()

Review Comment:
   remove the mapping



-- 
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]

Reply via email to