pkarashchenko commented on code in PR #10380:
URL: https://github.com/apache/nuttx/pull/10380#discussion_r1306009473
##########
binfmt/binfmt_execmodule.c:
##########
@@ -96,6 +97,80 @@ static void exec_ctors(FAR void *arg)
}
#endif
+/****************************************************************************
+ * Name: exchange_pid
+ *
+ * Description:
+ * Exchange the pid of tasks, and reverse parent-child relationship.
+ *
+ * Input Parameters:
+ * ptcb - parent task tcb.
+ * chtcb - child task tcb.
+ *
+ * Returned Value:
+ * NULL
Review Comment:
```suggestion
* none
```
##########
binfmt/binfmt_execmodule.c:
##########
@@ -96,6 +97,80 @@ static void exec_ctors(FAR void *arg)
}
#endif
+/****************************************************************************
+ * Name: exchange_pid
+ *
+ * Description:
+ * Exchange the pid of tasks, and reverse parent-child relationship.
+ *
+ * Input Parameters:
+ * ptcb - parent task tcb.
+ * chtcb - child task tcb.
+ *
+ * Returned Value:
+ * NULL
+ *
+ ****************************************************************************/
+
+static void exchange_pid(FAR struct tcb_s * ptcb, FAR struct tcb_s * chtcb)
Review Comment:
```suggestion
static void exchange_pid(FAR struct tcb_s *ptcb, FAR struct tcb_s *chtcb)
```
##########
binfmt/binfmt_execmodule.c:
##########
@@ -96,6 +97,80 @@ static void exec_ctors(FAR void *arg)
}
#endif
+/****************************************************************************
+ * Name: exchange_pid
+ *
+ * Description:
+ * Exchange the pid of tasks, and reverse parent-child relationship.
+ *
+ * Input Parameters:
+ * ptcb - parent task tcb.
+ * chtcb - child task tcb.
+ *
+ * Returned Value:
+ * NULL
+ *
+ ****************************************************************************/
+
+static void exchange_pid(FAR struct tcb_s * ptcb, FAR struct tcb_s * chtcb)
+{
+ pid_t pid;
+ int pndx;
+ int chndx;
+
+ DEBUGASSERT(ptcb);
+ DEBUGASSERT(chtcb);
+
+ irqstate_t flags = enter_critical_section();
+
+ pndx = PIDHASH(ptcb->pid);
+ chndx = PIDHASH(chtcb->pid);
+
+ DEBUGASSERT(g_pidhash[pndx]);
+ DEBUGASSERT(g_pidhash[chndx]);
+
+ /* Exchange g_pidhash index */
+
+ g_pidhash[pndx] = chtcb;
+ g_pidhash[chndx] = ptcb;
+
+ /* Exchange pid */
+
+ pid = chtcb->pid;
+ chtcb->pid = ptcb->pid;
+ ptcb->pid = pid;
+
+ /* Exchange group info. This will reverse parent-child relationship */
+
+ pid = chtcb->group->tg_pid;
+ chtcb->group->tg_pid = ptcb->group->tg_pid;
+ ptcb->group->tg_pid = pid;
+
+ pid = chtcb->group->tg_ppid;
+ chtcb->group->tg_ppid = ptcb->group->tg_ppid;
+ ptcb->group->tg_ppid = pid;
+
+#ifdef HAVE_GROUP_MEMBERS
+ FAR pid_t *tg_members = chtcb->group->tg_members;
+ chtcb->group->tg_members = ptcb->group->tg_members;
+ ptcb->group->tg_members = tg_members;
+#endif
+
+#ifdef CONFIG_SCHED_HAVE_PARENT
+#ifdef CONFIG_SCHED_CHILD_STATUS
+ FAR struct child_status_s *tg_children = chtcb->group->tg_children;
+ chtcb->group->tg_children = ptcb->group->tg_children;
+ ptcb->group->tg_children = tg_children;
+#else
+ uint16_t tg_nchildren = chtcb->group->tg_nchildren;
Review Comment:
Move declaration of `tg_nchildren` to the beginning of the function
##########
binfmt/binfmt_execmodule.c:
##########
@@ -116,7 +191,8 @@ static void exec_ctors(FAR void *arg)
int exec_module(FAR struct binary_s *binp,
FAR const char *filename, FAR char * const *argv,
FAR char * const *envp,
- FAR const posix_spawn_file_actions_t *actions)
+ FAR const posix_spawn_file_actions_t *actions,
+ int exec_inner)
Review Comment:
Why `exec_inner` is `int` and not `bool`?
##########
binfmt/binfmt_execmodule.c:
##########
@@ -96,6 +97,80 @@ static void exec_ctors(FAR void *arg)
}
#endif
+/****************************************************************************
+ * Name: exchange_pid
+ *
+ * Description:
+ * Exchange the pid of tasks, and reverse parent-child relationship.
+ *
+ * Input Parameters:
+ * ptcb - parent task tcb.
+ * chtcb - child task tcb.
+ *
+ * Returned Value:
+ * NULL
+ *
+ ****************************************************************************/
+
+static void exchange_pid(FAR struct tcb_s * ptcb, FAR struct tcb_s * chtcb)
+{
+ pid_t pid;
+ int pndx;
+ int chndx;
+
+ DEBUGASSERT(ptcb);
+ DEBUGASSERT(chtcb);
+
+ irqstate_t flags = enter_critical_section();
+
+ pndx = PIDHASH(ptcb->pid);
+ chndx = PIDHASH(chtcb->pid);
+
+ DEBUGASSERT(g_pidhash[pndx]);
+ DEBUGASSERT(g_pidhash[chndx]);
+
+ /* Exchange g_pidhash index */
+
+ g_pidhash[pndx] = chtcb;
+ g_pidhash[chndx] = ptcb;
+
+ /* Exchange pid */
+
+ pid = chtcb->pid;
+ chtcb->pid = ptcb->pid;
+ ptcb->pid = pid;
+
+ /* Exchange group info. This will reverse parent-child relationship */
+
+ pid = chtcb->group->tg_pid;
+ chtcb->group->tg_pid = ptcb->group->tg_pid;
+ ptcb->group->tg_pid = pid;
+
+ pid = chtcb->group->tg_ppid;
+ chtcb->group->tg_ppid = ptcb->group->tg_ppid;
+ ptcb->group->tg_ppid = pid;
+
+#ifdef HAVE_GROUP_MEMBERS
+ FAR pid_t *tg_members = chtcb->group->tg_members;
Review Comment:
Move declaration of `tg_members` to the beginning of the function
##########
binfmt/binfmt_execmodule.c:
##########
@@ -96,6 +97,80 @@ static void exec_ctors(FAR void *arg)
}
#endif
+/****************************************************************************
+ * Name: exchange_pid
+ *
+ * Description:
+ * Exchange the pid of tasks, and reverse parent-child relationship.
+ *
+ * Input Parameters:
+ * ptcb - parent task tcb.
+ * chtcb - child task tcb.
+ *
+ * Returned Value:
+ * NULL
+ *
+ ****************************************************************************/
+
+static void exchange_pid(FAR struct tcb_s * ptcb, FAR struct tcb_s * chtcb)
+{
+ pid_t pid;
+ int pndx;
+ int chndx;
+
+ DEBUGASSERT(ptcb);
+ DEBUGASSERT(chtcb);
+
+ irqstate_t flags = enter_critical_section();
Review Comment:
```suggestion
int chndx;
irqstate_t flags;
DEBUGASSERT(ptcb);
DEBUGASSERT(chtcb);
flags = enter_critical_section();
```
##########
binfmt/binfmt_execmodule.c:
##########
@@ -96,6 +97,80 @@ static void exec_ctors(FAR void *arg)
}
#endif
+/****************************************************************************
+ * Name: exchange_pid
+ *
+ * Description:
+ * Exchange the pid of tasks, and reverse parent-child relationship.
+ *
+ * Input Parameters:
+ * ptcb - parent task tcb.
+ * chtcb - child task tcb.
+ *
+ * Returned Value:
+ * NULL
+ *
+ ****************************************************************************/
+
+static void exchange_pid(FAR struct tcb_s * ptcb, FAR struct tcb_s * chtcb)
+{
+ pid_t pid;
+ int pndx;
+ int chndx;
+
+ DEBUGASSERT(ptcb);
+ DEBUGASSERT(chtcb);
+
+ irqstate_t flags = enter_critical_section();
+
+ pndx = PIDHASH(ptcb->pid);
+ chndx = PIDHASH(chtcb->pid);
Review Comment:
```suggestion
pndx = PIDHASH(ptcb->pid);
chndx = PIDHASH(chtcb->pid);
```
or
```suggestion
pndx = PIDHASH(ptcb->pid);
chndx = PIDHASH(chtcb->pid);
```
##########
binfmt/binfmt_execmodule.c:
##########
@@ -96,6 +97,80 @@ static void exec_ctors(FAR void *arg)
}
#endif
+/****************************************************************************
+ * Name: exchange_pid
+ *
+ * Description:
+ * Exchange the pid of tasks, and reverse parent-child relationship.
+ *
+ * Input Parameters:
+ * ptcb - parent task tcb.
+ * chtcb - child task tcb.
+ *
+ * Returned Value:
+ * NULL
+ *
+ ****************************************************************************/
+
+static void exchange_pid(FAR struct tcb_s * ptcb, FAR struct tcb_s * chtcb)
+{
+ pid_t pid;
+ int pndx;
+ int chndx;
+
+ DEBUGASSERT(ptcb);
+ DEBUGASSERT(chtcb);
+
+ irqstate_t flags = enter_critical_section();
+
+ pndx = PIDHASH(ptcb->pid);
+ chndx = PIDHASH(chtcb->pid);
+
+ DEBUGASSERT(g_pidhash[pndx]);
+ DEBUGASSERT(g_pidhash[chndx]);
+
+ /* Exchange g_pidhash index */
+
+ g_pidhash[pndx] = chtcb;
+ g_pidhash[chndx] = ptcb;
+
+ /* Exchange pid */
+
+ pid = chtcb->pid;
+ chtcb->pid = ptcb->pid;
+ ptcb->pid = pid;
+
+ /* Exchange group info. This will reverse parent-child relationship */
+
+ pid = chtcb->group->tg_pid;
+ chtcb->group->tg_pid = ptcb->group->tg_pid;
+ ptcb->group->tg_pid = pid;
+
+ pid = chtcb->group->tg_ppid;
+ chtcb->group->tg_ppid = ptcb->group->tg_ppid;
+ ptcb->group->tg_ppid = pid;
+
+#ifdef HAVE_GROUP_MEMBERS
+ FAR pid_t *tg_members = chtcb->group->tg_members;
+ chtcb->group->tg_members = ptcb->group->tg_members;
+ ptcb->group->tg_members = tg_members;
+#endif
+
+#ifdef CONFIG_SCHED_HAVE_PARENT
+#ifdef CONFIG_SCHED_CHILD_STATUS
+ FAR struct child_status_s *tg_children = chtcb->group->tg_children;
Review Comment:
Move declaration of `tg_children` to the beginning of the function
--
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]
