Re: [PATCH 3/5] write_file(): introduce an explicit WRITE_FILE_GENTLY request

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 24, 2015, 18:41 UTC
Message-ID: <xmqqh9no4jhk.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1440436186-7894-4-git-send-email-gitster@pobox.com>

Junio C Hamano <gitster@pobox.com> writes:

> All callers except for two ask this function to die upon error by
> passing fatal=1; turn the parameter to a more generic "unsigned flag"
> bag of bits, introduce an explicit WRITE_FILE_GENTLY bit and change
> these two callers to pass that bit.

There is a huge iffyness around one of these two oddball callers.

Show 12 quoted lines

> diff --git a/setup.c b/setup.c
> index 5f9f07d..718f4e1 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -404,7 +404,7 @@ static void update_linked_gitdir(const char *gitfile, const char *gitdir)
>  
>  	strbuf_addf(&path, "%s/gitfile", gitdir);
>  	if (stat(path.buf, &st) || st.st_mtime + 24 * 3600 < time(NULL))
> -		write_file(path.buf, 0, "%s\n", gitfile);
> +		write_file(path.buf, WRITE_FILE_GENTLY, "%s\n", gitfile);
>  	strbuf_release(&path);
>  }

This comes from 23af91d1 (prune: strategies for linked checkouts, 2014-11-30). I cannot tell what the justification is to treat a failure to write a gitfile as a non-error event. Just a sloppy coding that lets the program go through to its finish, ignoring the harm done by possibly corrupting user repository silently?

Show 13 quoted lines

> diff --git a/transport.c b/transport.c
> index 40692f8..e1821a4 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -291,7 +291,7 @@ static int write_one_ref(const char *name, const struct object_id *oid,
>  
>  	strbuf_addstr(buf, name);
>  	if (safe_create_leading_directories(buf->buf) ||
> -	    write_file(buf->buf, 0, "%s\n", oid_to_hex(oid)))
> +	    write_file(buf->buf, WRITE_FILE_GENTLY, "%s\n", oid_to_hex(oid)))
>  		return error("problems writing temporary file %s: %s",
>  			     buf->buf, strerror(errno));
>  	strbuf_setlen(buf, len);

This one is OK, in that it is merely to give a better error diagnosis than just "oh, I cannot write so I die".