On 4/1/21 11:40 AM, Jeff Hostetler via GitGitGadget wrote: Here is a rather important bug that I found on a whim while working with sparse-index integrations. The sparse-index isn't important except that it caused a different pattern of batch creation and responses from the daemon. > +/* > + * Format an opaque token string to send to the client. > + */ > +static void fsmonitor_format_response_token( > + struct strbuf *response_token, > + const struct strbuf *response_token_id, > + const struct fsmonitor_batch *batch) > +{ > + uint64_t seq_nr = (batch) ? batch->batch_seq_nr + 1 : 0; Here, you add one to the batch value to indicate a difference between "zero" and "positive" values. > + > + strbuf_reset(response_token); > + strbuf_addf(response_token, "builtin:%s:%"PRIu64, > + response_token_id->buf, seq_nr); > +} > + > +/* > + * Parse an opaque token from the client. > + */ > +static int fsmonitor_parse_client_token(const char *buf_token, > + struct strbuf *requested_token_id, > + uint64_t *seq_nr) > +{ > + const char *p; > + char *p_end; > + > + strbuf_reset(requested_token_id); > + *seq_nr = 0; > + > + if (!skip_prefix(buf_token, "builtin:", &p)) > + return 1; > + > + while (*p && *p != ':') > + strbuf_addch(requested_token_id, *p++); > + if (!*p++) > + return 1; > + > + *seq_nr = (uint64_t)strtoumax(p, &p_end, 10); Which means here you should decrement one from the value, possibly, (except if it is zero). > + if (*p_end) > + return 1; > + > + return 0; > +} ... > + shown = kh_init_str(); > + for (batch = batch_head; > + batch && batch->batch_seq_nr >= requested_oldest_seq_nr; > + batch = batch->next) { And without either decrementing one from requested_oldest_seq_nr or adding one to the batch_seq_nr here, this loop could terminate immediately. In my testing, I added one to the left-hand side of the inequality. Thanks, -Stolee