[PATCH] nscd: don't fork twice

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] nscd: don't fork twice

Andreas Schwab
There is no need to fork twice when daemonizing.

Andreas.

        * nscd/nscd.c (main): Don't fork again after closing files.

diff --git a/nscd/nscd.c b/nscd/nscd.c
index 26cf3c2..ffbc6f8 100644
--- a/nscd/nscd.c
+++ b/nscd/nscd.c
@@ -252,15 +252,6 @@ main (int argc, char **argv)
  for (i = min_close_fd; i < getdtablesize (); i++)
   close (i);
 
-      if (run_mode == RUN_DAEMONIZE)
- {
-  pid = fork ();
-  if (pid == -1)
-    error (EXIT_FAILURE, errno, _("cannot fork"));
-  if (pid != 0)
-    exit (0);
- }
-
       setsid ();
 
       if (chdir ("/") != 0)
--
1.8.1

--
Andreas Schwab, SUSE Labs, [hidden email]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nscd: don't fork twice

Carlos O'Donell-2
On 01/14/2013 05:53 AM, Andreas Schwab wrote:

> There is no need to fork twice when daemonizing.
>
> Andreas.
>
> * nscd/nscd.c (main): Don't fork again after closing files.
>
> diff --git a/nscd/nscd.c b/nscd/nscd.c
> index 26cf3c2..ffbc6f8 100644
> --- a/nscd/nscd.c
> +++ b/nscd/nscd.c
> @@ -252,15 +252,6 @@ main (int argc, char **argv)
>   for (i = min_close_fd; i < getdtablesize (); i++)
>    close (i);
>  
> -      if (run_mode == RUN_DAEMONIZE)
> - {
> -  pid = fork ();
> -  if (pid == -1)
> -    error (EXIT_FAILURE, errno, _("cannot fork"));
> -  if (pid != 0)
> -    exit (0);
> - }
> -
>        setsid ();
>  
>        if (chdir ("/") != 0)
>

Looks good to me.

I'm a little confused though, and I haven't checked exactly what's
going on, but it looks like you reverted a bunch of patches and are
sending them out for review. What's the story there?

Cheers,
Carlos.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nscd: don't fork twice

Florian Weimer-5
In reply to this post by Andreas Schwab
On 01/14/2013 11:53 AM, Andreas Schwab wrote:

> There is no need to fork twice when daemonizing.
>
> Andreas.
>
> * nscd/nscd.c (main): Don't fork again after closing files.
>
> diff --git a/nscd/nscd.c b/nscd/nscd.c
> index 26cf3c2..ffbc6f8 100644
> --- a/nscd/nscd.c
> +++ b/nscd/nscd.c
> @@ -252,15 +252,6 @@ main (int argc, char **argv)
>   for (i = min_close_fd; i < getdtablesize (); i++)
>    close (i);
>
> -      if (run_mode == RUN_DAEMONIZE)
> - {
> -  pid = fork ();
> -  if (pid == -1)
> -    error (EXIT_FAILURE, errno, _("cannot fork"));
> -  if (pid != 0)
> -    exit (0);
> - }
> -
>         setsid ();

Could you add error checking to the setsid call?  I checked and it seems
that a single fork is enough on Linux.  The double-fork idiom is
supposed to be a SYSV-ism, but perhaps it's required on FreeBSD or Hurd,
too.

--
Florian Weimer / Red Hat Product Security Team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nscd: don't fork twice

Andreas Schwab
In reply to this post by Carlos O'Donell-2
Carlos O'Donell <[hidden email]> writes:

> I'm a little confused though, and I haven't checked exactly what's
> going on, but it looks like you reverted a bunch of patches and are
> sending them out for review. What's the story there?

That was an accident, I've pushed the wrong branch.

Andreas.

--
Andreas Schwab, SUSE Labs, [hidden email]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nscd: don't fork twice

Carlos O'Donell-2
On 01/14/2013 10:04 AM, Andreas Schwab wrote:
> Carlos O'Donell <[hidden email]> writes:
>
>> I'm a little confused though, and I haven't checked exactly what's
>> going on, but it looks like you reverted a bunch of patches and are
>> sending them out for review. What's the story there?
>
> That was an accident, I've pushed the wrong branch.

Thanks for clarifying.

Cheers,
Carlos.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nscd: don't fork twice

Andreas Schwab
In reply to this post by Florian Weimer-5
Florian Weimer <[hidden email]> writes:

> Could you add error checking to the setsid call?

Which failure modes are possible, apart from the fact that it will
always fail in foreground mode?

Andreas.

        * nscd/nscd.c (main): Don't fork again after closing files.  Don't
        call setsid in foreground mode.

diff --git a/nscd/nscd.c b/nscd/nscd.c
index 26cf3c2..4e527ff 100644
--- a/nscd/nscd.c
+++ b/nscd/nscd.c
@@ -252,16 +252,10 @@ main (int argc, char **argv)
  for (i = min_close_fd; i < getdtablesize (); i++)
   close (i);
 
+      /* In foreground mode this would always fail, we assume our caller
+ has already established a new session.  */
       if (run_mode == RUN_DAEMONIZE)
- {
-  pid = fork ();
-  if (pid == -1)
-    error (EXIT_FAILURE, errno, _("cannot fork"));
-  if (pid != 0)
-    exit (0);
- }
-
-      setsid ();
+ setsid ();
 
       if (chdir ("/") != 0)
  error (EXIT_FAILURE, errno,
--
1.8.1

--
Andreas Schwab, SUSE Labs, [hidden email]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."