Re: [PATCH v3 3/3] reftable: prevent 'update_index' changes after adding records

From: Patrick Steinhardt <ps@pks.im>
Date: Jan 22, 2025, 12:12 UTC
Message-ID: <Z5DgxQuc2j_-5GHg@pks.im>
In-Reply-To: <20250122-461-corrupted-reftable-followup-v3-3-ae5f88bf04fa@gmail.com>

On Wed, Jan 22, 2025 at 06:35:49AM +0100, Karthik Nayak wrote:

Show 15 quoted lines

> diff --git a/t/unit-tests/t-reftable-stack.c b/t/unit-tests/t-reftable-stack.c
> index aeec195b2b1014445d71c5db39a9795017fd8ff2..c3f0059c346edbe1ad543c9832959c6fc0aa9180 100644
> --- a/t/unit-tests/t-reftable-stack.c
> +++ b/t/unit-tests/t-reftable-stack.c
> @@ -1369,11 +1371,57 @@ static void t_reftable_stack_reload_with_missing_table(void)
>  	clear_dir(dir);
>  }
>  
> +static int write_limits_after_ref(struct reftable_writer *wr, void *arg)
> +{
> +	struct reftable_ref_record *ref = arg;
> +	check(!reftable_writer_set_limits(wr, ref->update_index, ref->update_index));
> +	check(!reftable_writer_add_ref(wr, ref));
> +	return reftable_writer_set_limits(wr, ref->update_index, ref->update_index);
> +}

Nice.

Show 11 quoted lines

> +static void t_reftable_invalid_limit_updates(void)
> +{
> +	struct reftable_ref_record ref = {
> +		.refname = (char *) "HEAD",
> +		.update_index = 1,
> +		.value_type = REFTABLE_REF_SYMREF,
> +		.value.symref = (char *) "master",
> +	};
> +	struct reftable_write_options opts = {
> +		.default_permissions = 0660,
> +	};

Nit: it's unnecessary to pass write options. Other than that the test
looks good to me, and this nit isn't worth a reroll.

Thanks for working on this!

Patrick