nightt5879 commented on code in PR #3511:
URL: https://github.com/apache/nuttx-apps/pull/3511#discussion_r3327893627
##########
system/popen/popen.c:
##########
@@ -45,17 +46,78 @@
* Private Types
****************************************************************************/
-/* struct popen_file_s is a cast compatible version of FILE that contains
- * the additional PID of the shell processes needed by pclose().
- */
-
struct popen_file_s
{
- FILE copy;
- FILE *original;
+ FAR struct popen_file_s *flink;
+ FAR FILE *stream;
pid_t shell;
};
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static sem_t g_popen_sem = SEM_INITIALIZER(1);
+static FAR struct popen_file_s *g_popen_files;
Review Comment:
Thanks for the review. I trust your judgment here.
The global list was added because once `popen()` returns the real `fdopen()`
stream, `pclose(FILE *stream)` still needs a way to recover the shell pid for
`waitpid()`.
The alternatives I can see are:
1. keep external `stream/fd -> pid` state, as this PR does;
2. store popen metadata in NuttX `FILE`/stdio internals;
3. keep the old copied-`FILE` approach;
4. make a broader libc/helper API change.
I chose (1) to avoid copying `FILE` or modifying libc internals, but I
understand this may not be the tradeoff you want for this issue.
What approach would you suggest here?
--
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]
