> From: Junio C Hamano [mailto:gitster@xxxxxxxxx] > Sent: Friday, August 24, 2012 6:44 PM > To: Joachim Schmitz > Cc: git@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v2] Prefer sysconf(_SC_OPEN_MAX) over getrlimit(RLIMIT_NOFILE,...) > > "Joachim Schmitz" <jojo@xxxxxxxxxxxxxxxxxx> writes: > > > Signed-off-by: Joachim Schmitz <jojo@xxxxxxxxxxxxxxxxxx> > > --- > > As discussed now as a small helper function rather than #ifdef/#endif in the primary flow of the code. > > And hopefully without having screwed up whitespace and line breaks > > The formatting looks fine. > > Perhaps I am being overly paranoid, but I would prefer not to change > things for people who have been using getrlimit(). For them, if > they also have sysconf(_SC_OPEN_MAX), your code _ought to_ work, but > if it does not work for whatever reason (perhaps some platforms > claim to have both, but getrlimit() works and sysconf(_SC_OPEN_MAX) > is broken), it will given them an unnecessary regression. Sounds reasonable, so reasonable that I wonder why I didn't have that idea ;-) > So how about doing it this way instead? > > -- >8 -- > Subject: sha1_file.c: introduce get_max_fd_limit() helper > > Not all platforms have getrlimit(), but there are other ways to see > the maximum number of files that a process can have open. If > getrlimit() is unavailable, fall back to sysconf(_SC_OPEN_MAX) if > available, and use OPEN_MAX from <limits.h>. > > Signed-off-by: Joachim Schmitz <jojo@xxxxxxxxxxxxxxxxxx> > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > --- > sha1_file.c | 26 +++++++++++++++++++------- > 1 file changed, 19 insertions(+), 7 deletions(-) > > diff --git c/sha1_file.c w/sha1_file.c > index af5cfbd..9152974 100644 > --- c/sha1_file.c > +++ w/sha1_file.c > @@ -731,6 +731,24 @@ void free_pack_by_name(const char *pack_name) > } > } > > +static unsigned int get_max_fd_limit(void) > +{ > +#ifdef RLIMIT_NOFILE > + struct rlimit lim; > + > + if (getrlimit(RLIMIT_NOFILE, &lim)) > + die_errno("cannot get RLIMIT_NOFILE"); > + > + return lim.rlim_cur; > +#elif defined(_SC_OPEN_MAX) > + return sysconf(_SC_OPEN_MAX); > +#elif defined(OPEN_MAX) > + return OPEN_MAX; > +#else > + return 1; /* see the caller ;-) */ > +#endif > +} > + > /* > * Do not call this directly as this leaks p->pack_fd on error return; > * call open_packed_git() instead. > @@ -747,13 +765,7 @@ static int open_packed_git_1(struct packed_git *p) > return error("packfile %s index unavailable", p->pack_name); > > if (!pack_max_fds) { > - struct rlimit lim; > - unsigned int max_fds; > - > - if (getrlimit(RLIMIT_NOFILE, &lim)) > - die_errno("cannot get RLIMIT_NOFILE"); > - > - max_fds = lim.rlim_cur; > + unsigned int max_fds = get_max_fd_limit(); > > /* Save 3 for stdin/stdout/stderr, 22 for work */ > if (25 < max_fds) Looks good to me. Stupid newbie question: how would I revert my commit to my clone, to then add (and test) this one? Bye, Jojo -- 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