Re: [PATCH v4 15/15] daemon: opt-out on features that require posix
- From
Erik Faye-Lund <kusmabite@gmail.com>
- Date
- Oct 14, 2010, 11:02 UTC
- Message-ID
- <AANLkTi=N+4QokkiGmN+S17=7=QsgKWUXrsF0vZ_6Zt2H@mail.gmail.com>
- In-Reply-To
- <7vhbgphf25.fsf@alter.siamese.dyndns.org>
On Thu, Oct 14, 2010 at 1:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 33 quoted lines
> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
>> 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?
>To avoid having different signatures for the serve-function dependent on NO_POSIX_GOODIES.
Do you have any other suggestions on how to do this? Perhaps I should just move the logic in serve() to the end of main()? It's the only call-site for the function, and would remove the need for a function prototype all-together...
Show 14 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.
>Sure, makes sense.
> The same aversion to too many #ifdef's apply to the rest of the patch. >
I can remove some of them, like keeping the variables in main() around, even though they'll be constant. That might cause some compile-time warnings, though.