Re: [PATCH v2 9/9] reftable/stack: handle locked tables during auto-compaction
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Aug 7, 2024, 19:12 UTC
- Message-ID
- <cyvkxpz3ro5zq6san5jtjyhhdxhw6j4tu7fcubmzolo3xifdke@zohma4ybye7d>
- In-Reply-To
- <ZrMGyMTm3jXLSLtI@tanuki>
On 24/08/07 07:31AM, Patrick Steinhardt wrote:
Show 18 quoted lines
> On Tue, Aug 06, 2024 at 01:46:35PM -0500, Justin Tobler wrote:
> > 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 :)Ah, thanks for explaining! This version of the patch series looks good to me.
-Justin