Re: [PATCH v1 1/2] sha1_file: open window into packfiles with CLOEXEC

[Eric Wong <e@80x24.org>]

larsxschneider@gmail.com wrote:
> All processes that the Git main process spawns inherit the open file
> descriptors of the main process. These leaked file descriptors can
> cause problems.


> -int git_open_noatime(const char *name)
> +int git_open_noatime_cloexec(const char *name)
>  {
> -	static int sha1_file_open_flag = O_NOATIME;
> +	static int sha1_file_open_flag = O_NOATIME | O_CLOEXEC;
>  
>  	for (;;) {
>  		int fd;

If there's real problems being caused by lack of cloexec
today, I think the F_SETFD fallback I proposed in
https://public-inbox.org/git/20160818173555.GA29253@starla/
will be necessary.

I question the need for the "_cloexec" suffixing in the
function name since the old function is going away entirely.

I prefer all FD-creating functions set cloexec by default
for FD > 2 to avoid inadvertantly leaking FDs.  So we
ought to use pipe2, accept4, socket(..., SOCK_CLOEXEC), etc...
and fallback to the racy+slower F_SETFD when not available.


Fwiw, Perl has been setting cloexec on FDs above $^F
(2, $SYSTEM_FD_MAX) for decades, and Ruby started
doing it a few years ago, too.