nightt5879 commented on code in PR #3511:
URL: https://github.com/apache/nuttx-apps/pull/3511#discussion_r3338122983
##########
system/popen/popen.c:
##########
@@ -97,11 +97,37 @@ static off_t popen_file_seek(FAR void *cookie, FAR off_t
*offset,
static int popen_file_close(FAR void *cookie)
{
FAR struct popen_file_s *filep = (FAR struct popen_file_s *)cookie;
+ int errcode = OK;
int ret;
+#ifdef CONFIG_SCHED_WAITPID
+ int status;
+#endif
ret = close(filep->fd);
+ if (ret < 0)
+ {
+ errcode = errno;
+ }
+
+#ifdef CONFIG_SCHED_WAITPID
+ if (filep->shell > 0)
+ {
+ ret = waitpid(filep->shell, &status, 0);
+ if (ret < 0 && errcode == OK)
+ {
+ errcode = errno;
+ }
+ }
+#endif
+
free(filep);
- return ret;
+ if (errcode != OK)
+ {
+ errno = errcode;
Review Comment:
Done in 433405a4a. close() and waitpid() already leave errno on failure, so
popen_file_close() now returns ERROR directly without saving/restoring errno
manually.
##########
system/popen/popen.c:
##########
@@ -397,17 +425,20 @@ FILE *popen(FAR const char *command, FAR const char *mode)
errout_with_stream:
fclose(stream);
- container = NULL;
+ close(newfd[0]);
Review Comment:
fclose(stream) only closes retfd, which is owned by the returned FILE
stream. newfd[0] is the child-side descriptor prepared for dup2(), so it is
still owned by popen() on this error path and still needs to be closed
separately. I kept the close and added a short comment to make that ownership
split explicit.
--
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]
