Hi all,
V3 attached.
Rebased against master to account for the -d option. Moves the output
variable from the globals struct to be local to main.
Thanks,
Louai
On Tue, Nov 7, 2023 at 10:27 AM 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:
> > >
> > > On Wed, Nov 1, 2023 at 11:53 PM Louai Al-Khanji <lo...@astranis.com>
> > > wrote:
> > > >
> > > > Hi,
> > > >
> > > > I am interested in implementing the -o|--output option that the Debian
> > > > start-stop-daemon supports. Might such a patch be considered for
> > > > upstreaming?
> > > >
> > > > Thanks,
> > > > Louai
> > >
> > > Hello,
> > >
> > > 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.
>
> 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.
>
> Thanks,
> Louai
--
________
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.
From bee1ed58ae1b621dcb0d00143901a8fa558a6f55 Mon Sep 17 00:00:00 2001
From: Louai Al-Khanji <lo...@astranis.com>
Date: Fri, 3 Nov 2023 14:36:23 -0700
Subject: [PATCH] start-stop-daemon: implement option -O|--output
If specified redirect command stdout and stderr to given pathname.
---
debianutils/start_stop_daemon.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/debianutils/start_stop_daemon.c b/debianutils/start_stop_daemon.c
index 8573c6990..e8af5c5e5 100644
--- a/debianutils/start_stop_daemon.c
+++ b/debianutils/start_stop_daemon.c
@@ -76,6 +76,9 @@ Options which are valid for --start only:
"start-stop-daemon -S -n pwd -x /bin/pwd" works too.
"start-stop-daemon -S -a /bin/pwd -x /bin/pwd" works too.
Earlier versions were less picky (which?)
+ -O,--output PATHNAME Redirect stdout and stderr to PATHNAME. Only
+ relevant with --background.
+
Options which are valid for --stop only:
-s,--signal SIG Signal to send (default:TERM)
@@ -110,6 +113,7 @@ Misc options:
//config: -o|--oknodo ignored since we exit with 0 anyway
//config: -v|--verbose
//config: -N|--nicelevel N
+//config: -O|--output pathname
//applet:IF_START_STOP_DAEMON(APPLET_ODDNAME(start-stop-daemon, start_stop_daemon, BB_DIR_SBIN, BB_SUID_DROP, start_stop_daemon))
/* not NOEXEC: uses bb_common_bufsiz1 */
@@ -147,8 +151,9 @@ Misc options:
//usage: IF_FEATURE_START_STOP_DAEMON_FANCY(
//usage: "\n -o Exit with status 0 if nothing is done"
//usage: "\n -v Verbose"
-//usage: )
//usage: "\n -q Quiet"
+//usage: "\n -O pathname Redirect stdout and stderr to pathname"
+//usage: )
/* Override ENABLE_FEATURE_PIDFILE */
#define WANT_PIDFILE 1
@@ -178,6 +183,7 @@ enum {
OPT_OKNODO = (1 << 14) * ENABLE_FEATURE_START_STOP_DAEMON_FANCY, // -o
OPT_VERBOSE = (1 << 15) * ENABLE_FEATURE_START_STOP_DAEMON_FANCY, // -v
OPT_NICELEVEL = (1 << 16) * ENABLE_FEATURE_START_STOP_DAEMON_FANCY, // -N
+ OPT_OUTPUT = (1 << 17) * ENABLE_FEATURE_START_STOP_DAEMON_FANCY, // -O
};
#define QUIET (option_mask32 & OPT_QUIET)
#define TEST (option_mask32 & OPT_TEST)
@@ -420,6 +426,7 @@ static const char start_stop_daemon_longopts[] ALIGN1 =
"oknodo\0" No_argument "o"
"verbose\0" No_argument "v"
"nicelevel\0" Required_argument "N"
+ "output\0" Required_argument "O"
# endif
"startas\0" Required_argument "a"
"name\0" Required_argument "n"
@@ -452,13 +459,14 @@ int start_stop_daemon_main(int argc UNUSED_PARAM, char **argv)
// char *retry_arg = NULL;
// int retries = -1;
char *opt_N;
+ char *output;
#endif
INIT_G();
opt = GETOPT32(argv, "^"
"KSbqtma:n:s:u:c:d:x:p:"
- IF_FEATURE_START_STOP_DAEMON_FANCY("ovN:R:")
+ IF_FEATURE_START_STOP_DAEMON_FANCY("ovN:O:R:")
"\0"
"K:S:K--S:S--K"
/* -K or -S is required; they are mutually exclusive */
@@ -475,10 +483,16 @@ int start_stop_daemon_main(int argc UNUSED_PARAM, char **argv)
LONGOPTS
&startas, &cmdname, &signame, &userspec, &chuid, &chdir, &execname, &pidfile
IF_FEATURE_START_STOP_DAEMON_FANCY(,&opt_N)
+ IF_FEATURE_START_STOP_DAEMON_FANCY(,&output)
/* We accept and ignore -R <param> / --retry <param> */
IF_FEATURE_START_STOP_DAEMON_FANCY(,NULL)
);
+#ifdef ENABLE_FEATURE_START_STOP_DAEMON_FANCY
+ if ((opt & OPT_OUTPUT) && !(opt & OPT_BACKGROUND))
+ bb_show_usage();
+#endif
+
if (opt & OPT_s) {
signal_nr = get_signum(signame);
if (signal_nr < 0) bb_show_usage();
@@ -608,6 +622,14 @@ int start_stop_daemon_main(int argc UNUSED_PARAM, char **argv)
* strace -oLOG start-stop-daemon -S -x /bin/usleep -a qwerty 500000
* should exec "/bin/usleep", but argv[0] should be "qwerty":
*/
+#if ENABLE_FEATURE_START_STOP_DAEMON_FANCY
+ if (opt & OPT_OUTPUT) {
+ int outfd = xopen(output, O_WRONLY | O_CREAT | O_APPEND);
+ xdup2(outfd, STDOUT_FILENO);
+ xdup2(outfd, STDERR_FILENO);
+ xclose(outfd);
+ }
+#endif
execvp(execname, argv);
bb_perror_msg_and_die("can't execute '%s'", startas);
}
--
2.25.1
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
