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 55c28619761e242f06038397efeee5d471023f15 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 3e5dd9faa..ccccbff38 100644
--- a/debianutils/start_stop_daemon.c
+++ b/debianutils/start_stop_daemon.c
@@ -45,6 +45,8 @@ Options which are valid for --start only:
-c,--chuid USER[:[GRP]] Change to specified user [and group]
-m,--make-pidfile Write PID to the pidfile
(both -m and -p must be given!)
+ -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)
@@ -77,6 +79,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 */
@@ -113,8 +116,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
@@ -143,6 +147,7 @@ enum {
OPT_OKNODO = (1 << 13) * ENABLE_FEATURE_START_STOP_DAEMON_FANCY, // -o
OPT_VERBOSE = (1 << 14) * ENABLE_FEATURE_START_STOP_DAEMON_FANCY, // -v
OPT_NICELEVEL = (1 << 15) * ENABLE_FEATURE_START_STOP_DAEMON_FANCY, // -N
+ OPT_OUTPUT = (1 << 16) * ENABLE_FEATURE_START_STOP_DAEMON_FANCY, // -O
};
#define QUIET (option_mask32 & OPT_QUIET)
#define TEST (option_mask32 & OPT_TEST)
@@ -154,6 +159,7 @@ struct globals {
char *execname;
char *pidfile;
char *execname_cmpbuf;
+ char *output;
unsigned execname_sizeof;
int user_id;
smallint signal_nr;
@@ -168,6 +174,7 @@ struct globals {
#define pidfile (G.pidfile )
#define user_id (G.user_id )
#define signal_nr (G.signal_nr )
+#define output (G.output )
#define INIT_G() do { \
setup_common_bufsiz(); \
user_id = -1; \
@@ -385,6 +392,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"
@@ -421,7 +429,7 @@ int start_stop_daemon_main(int argc UNUSED_PARAM, char **argv)
opt = GETOPT32(argv, "^"
"KSbqtma:n:s:u:c:x:p:"
- IF_FEATURE_START_STOP_DAEMON_FANCY("ovN:R:")
+ IF_FEATURE_START_STOP_DAEMON_FANCY("ovN:O:R:")
/* -K or -S is required; they are mutually exclusive */
/* -p is required if -m is given */
/* -xpun (at least one) is required if -K is given */
@@ -434,10 +442,16 @@ int start_stop_daemon_main(int argc UNUSED_PARAM, char **argv)
LONGOPTS
&startas, &cmdname, &signame, &userspec, &chuid, &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();
@@ -564,6 +578,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
