Re: [PATCH 16/23] fsmonitor--daemon: implement handle_client callback
From: Derrick Stolee <hidden>
Date: 2021-05-13 18:52:24
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.
quoted hunk ↗ jump to hunk
+/* + * 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.
quoted hunk ↗ jump to hunk
+ + 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).
quoted hunk ↗ jump to hunk
+ if (*p_end) + return 1; + + return 0; +}
...
quoted hunk ↗ jump to hunk
+ 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