Re: [PATCH v4 15/15] daemon: opt-out on features that require posix

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 13, 2010, 23:02 UTC
Message-ID: <7vhbgphf25.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1286833829-5116-16-git-send-email-kusmabite@gmail.com>

Erik Faye-Lund <kusmabite@gmail.com> writes:

Show 28 quoted lines

> Windows does not supply the POSIX-functions fork(), setuuid(), setgid(),
> setsid() and initgroups(). Disable support for --user, --group and
> --detach if the NO_POSIX_GOODIES flag is set.
>
> MinGW doesn't have prototypes and headers for inet_ntop and inet_pton,
> so include our implementation instead. MSVC does have, so avoid doing
> so there.
>
> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> ---
> diff --git a/daemon.c b/daemon.c
> index 9b97b58..aa580f6 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -965,7 +969,12 @@ static void store_pid(const char *path)
>  		die_errno("failed to write pid file '%s'", path);
>  }
>  
> -static int serve(struct string_list *listen_addr, int listen_port, struct passwd *pass, gid_t gid)
> +#ifndef NO_POSIX_GOODIES
> +static struct passwd *pass;
> +static gid_t gid;
> +#endif
> +
> +static int serve(struct string_list *listen_addr, int listen_port)
>  {
>  	struct socketlist socklist = { NULL, 0, 0 };
>

This is ugly.  Why did you need to make the arguments file-scope static?

Show 10 quoted lines

> @@ -974,10 +983,12 @@ static int serve(struct string_list *listen_addr, int listen_port, struct passwd
>  		die("unable to allocate any listen sockets on port %u",
>  		    listen_port);
>  
> +#ifndef NO_POSIX_GOODIES
>  	if (pass && gid &&
>  	    (initgroups(pass->pw_name, gid) || setgid (gid) ||
>  	     setuid(pass->pw_uid)))
>  		die("cannot drop privileges");
> +#endif

It would be cleaner to make a helper (e.g. "drop-privileges") that is a no-op on NO_POSIX_GOODIES platform, and call that without #ifdef here.

The same aversion to too many #ifdef's apply to the rest of the patch.