Re: [PATCH] Add --foreground option for nscd

["Carlos O'Donell" <carlos at systemhalted dot org>]

On Wed, May 9, 2012 at 3:26 AM, Andreas Jaeger <aj@suse.de> wrote:
>
> For a better integration of nscd with systemd, we should run nscd not as
> daemon but in the foreground. A new --foreground option should be added.
>
> The following patch is taken from Fedora, I tested it on Linux/x86-64,
>
> Ok to commit?
>
> Andreas
>
> 2012-05-09 ?Alexandre Oliva ?<aoliva@redhat.com>
>
> ? ? ? ?* nscd/nscd.c (go_background): Replaced with...
> ? ? ? ?(run_in_foreground): ... this. ?Document it.
> ? ? ? ?(options): Add -F --foreground.
> ? ? ? ?(main): Implement it.
> ? ? ? ?(parse_opt): Parse it.
>
> diff --git a/nscd/nscd.c b/nscd/nscd.c
> index 9cd7273..77712bf 100644
> --- a/nscd/nscd.c
> +++ b/nscd/nscd.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 1998-2011, 2012 Free Software Foundation, Inc.
> +/* Copyright (c) 1998-2012 Free Software Foundation, Inc.
> ? ?This file is part of the GNU C Library.
> ? ?Contributed by Thorsten Kukuk <kukuk@suse.de>, 1998.
>
> @@ -71,7 +71,12 @@ thread_info_t thread_info;
> ?int do_shutdown;
> ?int disabled_passwd;
> ?int disabled_group;
> -int go_background = 1;
> +
> +/* Default is to daemonize. ?Set to 1 to run in foreground in
> + ? debugging mode, or negative to run in foreground but otherwise
> + ? behave like a daemon, i.e., detach from terminal and use
> + ? syslog. ?*/
> +static int run_in_foreground = 0;

Please use an enum. Magic constants are difficult to understand.

Everyone understands:

if (var == NAMED_STATE)

Nobody understands:

if (var <= 0)

... until they go and read the comment which describes two states at 1, and -1.

Am I the only one who doesn't like implied states?

> ?static const char *conffile = _PATH_NSCDCONF;
>
> @@ -103,6 +108,8 @@ static const struct argp_option options[] =
> ? ? N_("Read configuration data from NAME") },
> ? { "debug", 'd', NULL, 0,
> ? ? N_("Do not fork and display messages on the current tty") },
> + ?{ "foreground", 'F', NULL, 0,
> + ? ?N_("Do not fork, but otherwise behave like a deamon") },
> ? { "nthreads", 't', N_("NUMBER"), 0, N_("Start NUMBER threads") },
> ? { "shutdown", 'K', NULL, 0, N_("Shut the server down") },
> ? { "statistics", 'g', NULL, 0, N_("Print current configuration statistics") },
> @@ -173,16 +180,22 @@ main (int argc, char **argv)
> ? /* Determine page size. ?*/
> ? pagesize_m1 = getpagesize () - 1;
>
> - ?/* Behave like a daemon. ?*/
> - ?if (go_background)
> + ?if (run_in_foreground <= 0)
> ? ? {
> ? ? ? int i;
> + ? ? ?pid_t pid;
>
> - ? ? ?pid_t pid = fork ();
> - ? ? ?if (pid == -1)
> - ? ? ? error (EXIT_FAILURE, errno, _("cannot fork"));
> - ? ? ?if (pid != 0)
> - ? ? ? exit (0);
> + ? ? ?/* Behave like a daemon. ?*/
> + ? ? ?if (!run_in_foreground)
> + ? ? ? {
> + ? ? ? ? pid = fork ();
> + ? ? ? ? if (pid == -1)
> + ? ? ? ? ? error (EXIT_FAILURE, errno, _("cannot fork"));
> + ? ? ? ? if (pid != 0)
> + ? ? ? ? ? exit (0);
> + ? ? ? }
> + ? ? ?else
> + ? ? ? fprintf (stderr, _("further output sent to syslog\n"));
>
> ? ? ? int nullfd = open (_PATH_DEVNULL, O_RDWR);
> ? ? ? if (nullfd != -1)
> @@ -233,11 +246,14 @@ main (int argc, char **argv)
> ? ? ? ?for (i = min_close_fd; i < getdtablesize (); i++)
> ? ? ? ? ?close (i);
>
> - ? ? ?pid = fork ();
> - ? ? ?if (pid == -1)
> - ? ? ? error (EXIT_FAILURE, errno, _("cannot fork"));
> - ? ? ?if (pid != 0)
> - ? ? ? exit (0);
> + ? ? ?if (!run_in_foreground)
> + ? ? ? {
> + ? ? ? ? pid = fork ();
> + ? ? ? ? if (pid == -1)
> + ? ? ? ? ? error (EXIT_FAILURE, errno, _("cannot fork"));
> + ? ? ? ? if (pid != 0)
> + ? ? ? ? ? exit (0);
> + ? ? ? }
>
> ? ? ? setsid ();
>
> @@ -259,7 +275,7 @@ main (int argc, char **argv)
> ? ? ? signal (SIGTSTP, SIG_IGN);
> ? ? }
> ? else
> - ? ?/* In foreground mode we are not paranoid. ?*/
> + ? ?/* In debug mode we are not paranoid. ?*/
> ? ? paranoia = 0;
>
> ? signal (SIGINT, termination_handler);
> @@ -308,7 +324,11 @@ parse_opt (int key, char *arg, struct argp_state *state)
> ? ? {
> ? ? case 'd':
> ? ? ? ++debug_level;
> - ? ? ?go_background = 0;
> + ? ? ?run_in_foreground = 1;
> + ? ? ?break;
> +
> + ? ?case 'F':
> + ? ? ?run_in_foreground = -1;
> ? ? ? break;
>
> ? ? case 'f':
>
> --
> ?Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
> ?SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> ? GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
> ? ?GPG fingerprint = 93A3 365E CE47 B889 DF7F ?FED1 389A 563C C272 A126