Re: [PATCH v4 2/8] refs: add `index` field to `struct ref_udpate`

From: karthik nayak <karthik.188@gmail.com>
Date: Dec 20, 2024, 10:09 UTC
Message-ID: <CAOLa=ZRs3uy6ZyMeHCJUGgcDC3u9KXAymoK5SWD8_KJk3jxygw@mail.gmail.com>
In-Reply-To: <87bjx7h3q9.fsf@iotcl.com>

Toon Claes <toon@iotcl.com> writes:

Show 25 quoted lines

> Karthik Nayak <karthik.188@gmail.com> writes:
>> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
>> index e882602487c66261d586a94101bb1b4e9a2ed60e..c008f20be719fec3af6a8f81c821cb9c263764d7 100644
>> --- a/refs/reftable-backend.c
>> +++ b/refs/reftable-backend.c
>> @@ -1279,8 +1279,17 @@ static int reftable_be_transaction_abort(struct ref_store *ref_store UNUSED,
>>
>>  static int transaction_update_cmp(const void *a, const void *b)
>>  {
>> -	return strcmp(((struct reftable_transaction_update *)a)->update->refname,
>> -		      ((struct reftable_transaction_update *)b)->update->refname);
>> +	struct reftable_transaction_update *update_a = (struct reftable_transaction_update *)a;
>> +	struct reftable_transaction_update *update_b = (struct reftable_transaction_update *)b;
>> +
>> +	/*
>> +	 * If there is an index set, it should take preference (default is 0).
>> +	 * This ensures that updates with indexes are sorted amongst themselves.
>> +	 */
>> +	if (update_a->update->index || update_b->update->index)
>
> What if one of both simply isn't set, and the other one is? Then we
> compare an unset with one that is set? Or am I being too paranoid?
>
> --
> Toon

Those are expected scenarios, if one of them contains an index value,
then it'll be sorted before the other. At the end, we need:
1. Values with index to be sorted amongst themselves by index value.
2. Values without index to be sorted amongst themselves by the refname.

Karthik