Re: [PATCH 6/8] odb/source: support writing alternates when creating the database

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> Add the ability to write alternates when creating the object database.
> This change allows us to remove the `write_alternates()` callback in a
> subsequent patch.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---

> diff --git a/odb/source-files.c b/odb/source-files.c
> index b7b3a297bb..5e77b21d9f 100644
> --- a/odb/source-files.c
> +++ b/odb/source-files.c
> ...
> @@ -64,8 +70,71 @@ static int odb_source_files_create_on_disk(struct odb_source *source)
> +	if (opts->alternates && opts->alternates->nr) {
> +		strbuf_reset(&path);
> +		strbuf_addf(&path, "%s/info/alternates", source->path);
> +
> +		/*
> +		 * The alternates file may already exist, e.g. when it has been
> +		 * seeded from a template directory. Read any preexisting
> +		 * entries so that we don't end up writing duplicates.
> +		 */
> +		f = fopen(path.buf, "r");
> +		if (f) {
> +			while (strbuf_getline(&line, f) != EOF)
> +				strset_add(&seen, line.buf);
> +
> +			if (ferror(f)) {
> +				ret = error_errno(_("unable to read alternates file"));
> +				goto out;
> +			}
> +
> +			fclose(f);
> +		} else if (errno != ENOENT) {
> +			ret = error_errno(_("unable to read alternates file"));
> +			goto out;
> +		}
> +
> +		f = fopen(path.buf, "a");
> +		if (!f) {
> +			ret = error_errno(_("unable to open alternates file for writing"));
> +			goto out;
> +		}

I understand that using 'a' instead of 'w' is an attempt to deal
with the potential TOCTOU problem, but shouldn't we be using the
standard lockfile API, which atomically adds (or fails to add) to
avoid leaving a partially written file?  Or does it not matter,
since this is done only once upon repository creation when nobody
should be looking at the files on the filesystem?

Thanks.