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

[Andreas Jaeger <aj at suse dot com>]

On 05/09/2012 08:30 PM, Roland McGrath wrote:
>> +enum
>> +{
>> +  /* Running in foreground but otherwise behave like a daemon,
>> +     i.e., detach from terminal and use syslog.  */
>> +  RUN_FOREGROUND,
>> +  /* Running in background as daemon.  */
>> +  RUN_DAEMONIZE,
>> +  /* Run in foreground in debug mode.  */
>> +  RUN_DEBUG
>> +};
>> +static int run_mode = RUN_DAEMONIZE;
> 
> Make it a named typed and make the variable be of that type.
> A bit in the vein of microoptimization, but also what seems
> to me normal style for an enum, make the default value be the
> first (0) one.

Ok.

> I recall reviewing this when Alex posted it originally but I don't
> recall the details.  Please check the archives and make sure that
> all the review responses from then are addressed in the current
> version of the patch.

I did this now and removed the printf. Here's the patch in addition to
what I've committed already.

The open question I noticed during the review was about the
option. --foreground seems ok but should we remove the -F?

Are the changes below ok?

Andreas

2012-05-09  Andreas Jaeger  <aj@suse.de>

	* nscd/nscd.c (run_modes): Make named enum, reorder so that
	default is first entry.
	(run_mode): Set type.
	(main): Remove informal message about syslog.
	(options): Fix typo.

diff --git a/nscd/nscd.c b/nscd/nscd.c
index e22d8e8..79fb32f 100644
--- a/nscd/nscd.c
+++ b/nscd/nscd.c
@@ -72,17 +72,19 @@ int do_shutdown;
 int disabled_passwd;
 int disabled_group;
 
-static enum
+typedef enum
 {
+  /* Running in background as daemon.  */
+  RUN_DAEMONIZE,
   /* Running in foreground but otherwise behave like a daemon,
      i.e., detach from terminal and use syslog.  This allows
      better integration with services like systemd.  */
   RUN_FOREGROUND,
-  /* Running in background as daemon.  */
-  RUN_DAEMONIZE,
   /* Run in foreground in debug mode.  */
   RUN_DEBUG
-} run_mode = RUN_DAEMONIZE;
+} run_modes;
+
+static run_modes run_mode = RUN_DAEMONIZE;
 
 static const char *conffile = _PATH_NSCDCONF;
 
@@ -115,7 +117,7 @@ static const struct argp_option options[] =
   { "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") },
+    N_("Do not fork, but otherwise behave like a daemon") },
   { "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") },
@@ -200,8 +202,6 @@ main (int argc, char **argv)
 	  if (pid != 0)
 	    exit (0);
 	}
-      else
-	fprintf (stderr, _("further output sent to syslog\n"));
 
       int nullfd = open (_PATH_DEVNULL, O_RDWR);
       if (nullfd != -1)

-- 
 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