On Wed, Nov 8, 2023 at 5:08 AM Denys Vlasenko <vda.li...@googlemail.com> wrote: > > On Tue, Nov 7, 2023 at 7:27 PM Louai Al-Khanji <lo...@astranis.com> wrote: > > On Tue, Nov 7, 2023 at 8:03 AM Denys Vlasenko <vda.li...@googlemail.com> > > wrote: > > > On Fri, Nov 3, 2023 at 11:31 PM Louai Al-Khanji <lo...@astranis.com> > > > wrote: > > > > Attached is a proposed patch. Any feedback would be appreciated. > > > > > > My experiments with ssd version 1.21.22 show that the file is opened with > > > O_CREAT|O_APPEND, and it does not allow -O without -b. > > > > > > If execv fails, error message goes to this file. > > > IOW: there is no need to save/restore old stderr fd. Just replace it > > > with the new fd > > > (and don't forget to not leak any extra open fds). > > > > Thank you for the feedback everyone. New version attached. > > > > It looked a little tricky to me to add the logic around > > bb_daemon_helper() since it closes open fds. Maybe I am missing > > something. > > > > The code now checks the args more strictly and prints usage if -O is > > given without -b. > > > > I dropped restoring of the stdout/stderr fds. I believe this patch > > cannot leak fds. > > > > One question I have is whether it's okay to lose error messages. On > > failure to open the output file I believe the error message currently > > goes into the void. Same if any of the dup2 calls or the close call > > fails. > > Yes, that's not good. > I tried to fix it in git now. Please try. >
The new behavior seems correct to me - if the output file cannot be opened an error is printed at invocation site. If the exec fails it goes to the requested output file. Thank you for adding the -O functionality, it is very handy for my use case. > > BTW I noticed that bb_daemon_helper() internally calls setsid() > > already, so the extra call in start_stop_daemon.c seems superfluous. I > > didn't however touch that in this patch. > > No: > > #define bb_daemon_helper(arg) bb_daemonize_or_rexec((arg) | > DAEMON_ONLY_SANITIZE, NULL) > > DAEMON_ONLY_SANITIZE bit prevents setsid() and daemonization (the vfork). Ah, I missed that the macro set the flag. I see you added a comment to catch future readers, thank you. -- ________ This email and any attachments may contain Astranis confidential and/or proprietary information governed by a non-disclosure agreement, and are intended solely for the individual or entity specified by the message. _______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox