Re: [PATCH v2 9/9] reftable/stack: handle locked tables during auto-compaction

From: Patrick Steinhardt <ps@pks.im>
Date: Aug 7, 2024, 05:31 UTC
Message-ID: <ZrMGyMTm3jXLSLtI@tanuki>
In-Reply-To: <sct3bl7ztqwyym3wqh3jvz7vefr5gk4mf6dhsaasdihszcnnlf@s23uyyiwwbu3>

On Tue, Aug 06, 2024 at 01:46:35PM -0500, Justin Tobler wrote:

Show 11 quoted lines

> On 24/08/05 03:08PM, Patrick Steinhardt wrote:
> >  	REFTABLE_CALLOC_ARRAY(table_locks, last - first + 1);
> > -	for (i = first; i <= last; i++) {
> > -		stack_filename(&table_name, st, reader_name(st->readers[i]));
> > +	for (i = last + 1; i > first; i--) {
> > +		stack_filename(&table_name, st, reader_name(st->readers[i - 1]));
> 
> I might be missing something, but why not set `i = last` and `i >=
> first`? It looks like everywhere we reference `i` we subtract one
> anyways. Since `last` is already at the starting index, it seems it
> would be a bit more straightforward.

You are missing the case where `first == 0`. With `first = 0`, `i >= first` would be truish when `i == 0` and thus we would continue to loop. We then execute `i--`, wrap around, and still have `i >= first`.

Thus, an endless loop is born :)

Patrick