Jeff King <p...@peff.net> writes:

> On Wed, Dec 18, 2013 at 01:37:24PM -0800, Junio C Hamano wrote:
>
>> Jeff King <p...@peff.net> writes:
>> 
>> > According to the POSIX quote above, it sounds like we could do:
>> >
>> >   #if defined (_SC_OPEN_MAX)
>> >   {
>> >           long max;
>> >           errno = 0;
>> >           max = sysconf(_SC_OPEN_MAX);
>> >           if (0 < max) /* got the limit */
>> >                   return max;
>> >           else if (!errno) /* unlimited, cast to int-max */
>> >                   return max;
>> >           /* otherwise, fall through */
>> >   }
>> >   #endif
>> >
>> > Obviously you could collapse the two branches of the conditional, though
>> > I think it deserves at least a comment to explain what is going on.
>> 
>> Yes, that is locally OK, but depending on how the caller behaves, we
>> might need to have an extra saved_errno dance here, which I didn't
>> want to get into...
>
> I think we are fine. The only caller is about to clobber errno by
> closing packs anyway.
>
> -Peff

OK.

-- >8 --
Subject: [PATCH] get_max_fd_limit(): fall back to OPEN_MAX upon 
getrlimit/sysconf failure

On broken systems where RLIMIT_NOFILE is visible by the compliers
but underlying getrlimit() system call does not behave, we used to
simply die() when we are trying to decide how many file descriptors
to allocate for keeping packfiles open.  Instead, allow the fallback
codepath to take over when we get such a failure from getrlimit().

The same issue exists with _SC_OPEN_MAX and sysconf(); restructure
the code in a similar way to prepare for a broken sysconf() as well.

Noticed-by: Joey Hess <j...@kitenet.net>
Helped-by: Jeff King <p...@peff.net>
Signed-off-by: Junio C Hamano <gits...@pobox.com>
---
 sha1_file.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 760dd60..288badd 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -807,15 +807,38 @@ void free_pack_by_name(const char *pack_name)
 static unsigned int get_max_fd_limit(void)
 {
 #ifdef RLIMIT_NOFILE
-       struct rlimit lim;
+       {
+               struct rlimit lim;
 
-       if (getrlimit(RLIMIT_NOFILE, &lim))
-               die_errno("cannot get RLIMIT_NOFILE");
+               if (!getrlimit(RLIMIT_NOFILE, &lim))
+                       return lim.rlim_cur;
+       }
+#endif
+
+#ifdef _SC_OPEN_MAX
+       {
+               long open_max = sysconf(_SC_OPEN_MAX);
+               if (0 < open_max)
+                       return open_max;
+               /*
+                * Otherwise, we got -1 for one of the two
+                * reasons:
+                *
+                * (1) sysconf() did not understand _SC_OPEN_MAX
+                *     and signaled an error with -1; or
+                * (2) sysconf() said there is no limit.
+                *
+                * We _could_ clear errno before calling sysconf() to
+                * tell these two cases apart and return a huge number
+                * in the latter case to let the caller cap it to a
+                * value that is not so selfish, but letting the
+                * fallback OPEN_MAX codepath take care of these cases
+                * is a lot simpler.
+                */
+       }
+#endif
 
-       return lim.rlim_cur;
-#elif defined(_SC_OPEN_MAX)
-       return sysconf(_SC_OPEN_MAX);
-#elif defined(OPEN_MAX)
+#ifdef OPEN_MAX
        return OPEN_MAX;
 #else
        return 1; /* see the caller ;-) */
-- 
1.8.5.2-297-g3e57c29

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to