Hi,
A response to the general questions.
Erik Faye-Lund wrote:
> On Fri, Oct 15, 2010 at 11:16 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Why does the signature even have to be different between the two to begin
>> with? I _think_ you have gid_t over there
>
> We don't, so this is the primary reason.
Just to throw an idea out: you can also do something like
#ifndef NO_POSIX_GOODIES
struct credentials {
};
#else
struct credentials {
struct passwd *pass;
gid_t gid;
}
#endif
and pass a pointer to credentials around.
> But also avoiding
> compilation-warnings is a secondary motivation.
(void) gid;
works for this.
> We do, so this becomes a bit of a hypothetical question. But would you
> seriously consider pretending to have a posix-feature less ugly than
> inlining a function that is only used once?
In general, yes.
Long functions can make code much, much more difficult to read. A
fake posix feature just requires some suspension of disbelief.
In this case (#ifdef-heavy main() vs opaque struct passwd), both
strike me as ugly.
> (I'm going a little off-topic here, I hope that's OK)
> I'm not too happy with some of the
> pretend-really-hard-to-be-posix-magic around in the Windows-port. In
> fact, I have some patches to reduce posixness in some areas, while
> getting rid of some code in mingw.c. Would such patches be welcome, or
> is pretend-to-be-posix the governing portability approach? In some
> cases, this comes at the expense of some performance (and quite a bit
> of added cludge), which is a bit contradictory to the Git design IMO.
Sometimes the best abstraction is the posix one and sometimes not. I
don't think this would contradict with your planned patches, unless
they introduce #ifdefs all over the place.
>> This is especially
>> true if you are making the "drop-privileges" part a helper function, no?
>
> I don't follow this part. What exactly becomes more true by having a
> drop-privileges function?
(See linux-2.6.git:Documentation/SubmittingPatches, section "#ifdefs
are ugly".)
The ideal: never an #ifdef within a function. (Well, the ideal is
no #ifdef-s in .c files, but that's harder to take seriously.)
#ifndef HAVE_POSIX_GOODIES
static int drop_privileges(...)
{
return error("--user and --group not supported on this platform");
}
#endif
static int drop_privileges(...)
{
...
do
something
...
}
#endif
would make serve() look like
static int serve(...)
{
int socknum, *socklist;
... setup socket ...
if (want to drop privileges) {
if (drop_privileges(...))
return -1;
}
return service_loop(socknum, socklist);
}
which should be quite readable even to a person only interested in the
!HAVE_POSIX_GOODIES case imho. With some code rearrangement it could
be made nicer. Now compare:
static int serve(...)
{
int socknum, *socklist;
... setup socket ...
#ifdef HAVE_POSIX_GOODIES
...
do
things
...
#endif
return service_loop(socknum, socklist);
}
Just my two cents. Sorry I do not have something more substantive to
say.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html