Hi William,
On Thu, Dec 28, 2017 at 09:17:28PM +0100, William Lallemand wrote:
> On Thu, Dec 28, 2017 at 12:54:27AM +0100, William Lallemand wrote:
> > I think that's better to open /dev/null and dup2 with 0, 1, 2 so we won't
> > have any
> > surprise with an fprintf(stderr, ".. anywhere in the code.
> >
>
> Hi,
>
> I made a patch which does exactly that
In fact it still doesn't fclose() the streams, which worries me a little bit
for the long term, because eventhough any printf() will end up being written
into /dev/null, it's still preferable to mark the FILE* as being closed and
only then reopen 0,1,2 to ensure they cannot be abusively reused. It will
also remove some confusion in strace by avoiding seeing some spurious fcntl()
or write(2, foo, strlen(foo)) being sent there caused by alerts for example.
What I would do would simply be to add the following line to your actual
patch :
> +/*
> + * This function dup2 the stdio FDs (0,1,2) with <fd>, then closes <fd>
> + * If <fd> < 0, it opens /dev/null and use it to dup
> + *
> + * In the case of chrooting, you have to open /dev/null before the chroot,
> and
> + * pass the <fd> to this function
> + */
> +static void stdio_quiet(int fd)
> +{
> +
> + if (fd < 0)
> + fd = open("/dev/null", O_RDWR, 0);
> +
> + if (fd > -1) {
+ fclose(stdin); fclose(stdout); fclose(stderr);
> + dup2(fd, 0);
> + dup2(fd, 1);
> + dup2(fd, 2);
> + close(fd);
> + return;
> + }
> +
> + ha_alert("Cannot open /dev/null\n");
> + exit(EXIT_FAILURE);
> +}
Otherwise I'm reasonably confident that this should be enough to close
all pending issues related to the master-worker now.
Willy
