Hi,
Jan Stary wrote on Mon, Feb 10, 2020 at 05:12:53PM +0100:
> The -r option of newsyslog(8) removes the requirement
> that newsyslog runs as root. Would it also make sense
> to not try to send the SIGHUP to syslogd in that case?
While i'm not sure that i want to take care of this patch,
given that i'm not quite sure what the point of the -r option
even is in the first place, i'd like to point out that you
are also removing the warning. Is that intentional?
Naively, getting a warning when files are rotated but the
daemon isn't notified seems useful to me, even if you kind
of requested it with -r.
In that case, wouldn't it make more sense to say something
like (untested, and no code auditing done)
else if (needroot == 0 || kill(pid, signal))
?
That would also avoid the hopeless attempt to send a signal,
but would still print the warning.
Yours,
Ingo
> Index: newsyslog.8
> ===================================================================
> RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.8,v
> retrieving revision 1.54
> diff -u -p -r1.54 newsyslog.8
> --- newsyslog.8 20 Jul 2017 18:39:16 -0000 1.54
> +++ newsyslog.8 10 Feb 2020 16:08:51 -0000
> @@ -124,7 +124,7 @@ Removes the restriction that
> must be running as root.
> Note that in this mode
> .Nm
> -will not be able to send a
> +will not try to send a
> .Dv SIGHUP
> signal to
> .Xr syslogd 8 .
> Index: newsyslog.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.c,v
> retrieving revision 1.112
> diff -u -p -r1.112 newsyslog.c
> --- newsyslog.c 28 Jun 2019 13:35:02 -0000 1.112
> +++ newsyslog.c 10 Feb 2020 16:08:51 -0000
> @@ -394,7 +394,7 @@ send_signal(char *pidfile, int signal)
> warnx("%s pid file: %s", err, pidfile);
> else if (noaction)
> (void)printf("kill -%s %ld\n", sys_signame[signal], (long)pid);
> - else if (kill(pid, signal))
> + else if (needroot && kill(pid, signal))
> warnx("warning - could not send SIG%s to PID from pid file %s",
> sys_signame[signal], pidfile);
> }
