nightt5879 commented on PR #3511:
URL: https://github.com/apache/nuttx-apps/pull/3511#issuecomment-4585762275

   @xiaoxiang781216 Thanks, I addressed the latest review comments in commit 
6a16a4d4d.\n\nThe follow-up changes:\n- changed the invalid-mode path to jump 
to errout_with_container, since no pipe fd exists there;\n- avoided closing the 
stream fd twice after fopencookie() has taken ownership;\n- removed the 
redundant NULL check before free();\n- moved fd close, waitpid(), and cookie 
free into popen_file_close(), so pclose() now calls fclose() directly.\n\nOne 
caveat I noticed while moving waitpid() into the fopencookie close callback: 
fclose() treats any non-OK close callback return as an EOF/error result. 
Because of that, popen_file_close() now returns OK after close() and waitpid() 
themselves succeed, and reports only close()/waitpid() failures as errors. 
Otherwise a normal nonzero shell termination status could be converted into an 
fclose() failure.\n\nI re-tested with:\n- checkpatch.sh -f 
apps/system/popen/popen.c\n- sim:nsh build with CONFIG_SYSTEM_POPEN=y and 
CONFIG_TESTING_POPEN
 _TEST=y\n- popen_test, which prints successful


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