| Message ID | 20240218114936.1121077-1-marcel@roethke.info (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rerere: fix crash in during clear | expand |
Hi > rerere: fix crash in during clear “in during clear”? Did you mean “during clear”? On Sun, Feb 18, 2024, at 12:49, Marcel Röthke wrote: > When rerere_clear is called, for instance when aborting a rebase, and > the current conflict does not have a pre or postimage recorded git > crashes with a SEGFAULT in has_rerere_resolution when accessing the > status member of struct rerere_dir. This happens because scan_rerere_dir > only allocates the status field in struct rerere_dir when a post or > preimage was found. In some cases a segfault may happen even if a post > or preimage was recorded if it was not for the variant of interest and > the number of the variant that is present is lower than the variant of > interest. > > This patch solves this by making sure the status field is large enough You can simplify “This patch solves this” to “Solve this”; see `SubmittingPatches` under “imperative-mood”. > to accommodate for the variant of interest so it can be accesses without > checking if it is large enough. “accessed” > > An alternative solution would be to always check before accessing the > status field, but I think the chosen solution aligns better with the > assumptions made elsewhere in the code. > > Signed-off-by: Marcel Röthke <marcel@roethke.info> > --- > rerere.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/rerere.c b/rerere.c > index ca7e77ba68..3973ccce37 100644 > --- a/rerere.c > +++ b/rerere.c > @@ -219,6 +219,9 @@ static void read_rr(struct repository *r, struct > string_list *rr) > buf.buf[hexsz] = '\0'; > id = new_rerere_id_hex(buf.buf); > id->variant = variant; > + /* make sure id->collection->status has enough space > + * for the variant we are interested in */ Multi-line comments should have the delimiters on separate lines from the text. See `CodingGuidelines` under “Multi-line comments”. > + fit_variant(id->collection, variant); > string_list_insert(rr, path)->util = id; > } > strbuf_release(&buf); > -- > 2.43.2
On 2024-02-18 14:02:42, Kristoffer Haugsbakk wrote: > > rerere: fix crash in during clear > > “in during clear”? Did you mean “during clear”? Yes, thanks. > On Sun, Feb 18, 2024, at 12:49, Marcel Röthke wrote: > > When rerere_clear is called, for instance when aborting a rebase, and > > the current conflict does not have a pre or postimage recorded git > > crashes with a SEGFAULT in has_rerere_resolution when accessing the > > status member of struct rerere_dir. This happens because scan_rerere_dir > > only allocates the status field in struct rerere_dir when a post or > > preimage was found. In some cases a segfault may happen even if a post > > or preimage was recorded if it was not for the variant of interest and > > the number of the variant that is present is lower than the variant of > > interest. > > > > This patch solves this by making sure the status field is large enough > > You can simplify “This patch solves this” to “Solve this”; see > `SubmittingPatches` under “imperative-mood”. done > > > to accommodate for the variant of interest so it can be accesses without > > checking if it is large enough. > > “accessed” > done > > > > An alternative solution would be to always check before accessing the > > status field, but I think the chosen solution aligns better with the > > assumptions made elsewhere in the code. > > > > Signed-off-by: Marcel Röthke <marcel@roethke.info> > > --- > > rerere.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/rerere.c b/rerere.c > > index ca7e77ba68..3973ccce37 100644 > > --- a/rerere.c > > +++ b/rerere.c > > @@ -219,6 +219,9 @@ static void read_rr(struct repository *r, struct > > string_list *rr) > > buf.buf[hexsz] = '\0'; > > id = new_rerere_id_hex(buf.buf); > > id->variant = variant; > > + /* make sure id->collection->status has enough space > > + * for the variant we are interested in */ > > Multi-line comments should have the delimiters on separate lines from > the text. See `CodingGuidelines` under “Multi-line comments”. done Thank you for your feedback!
diff --git a/rerere.c b/rerere.c index ca7e77ba68..3973ccce37 100644 --- a/rerere.c +++ b/rerere.c @@ -219,6 +219,9 @@ static void read_rr(struct repository *r, struct string_list *rr) buf.buf[hexsz] = '\0'; id = new_rerere_id_hex(buf.buf); id->variant = variant; + /* make sure id->collection->status has enough space + * for the variant we are interested in */ + fit_variant(id->collection, variant); string_list_insert(rr, path)->util = id; } strbuf_release(&buf);
When rerere_clear is called, for instance when aborting a rebase, and the current conflict does not have a pre or postimage recorded git crashes with a SEGFAULT in has_rerere_resolution when accessing the status member of struct rerere_dir. This happens because scan_rerere_dir only allocates the status field in struct rerere_dir when a post or preimage was found. In some cases a segfault may happen even if a post or preimage was recorded if it was not for the variant of interest and the number of the variant that is present is lower than the variant of interest. This patch solves this by making sure the status field is large enough to accommodate for the variant of interest so it can be accesses without checking if it is large enough. An alternative solution would be to always check before accessing the status field, but I think the chosen solution aligns better with the assumptions made elsewhere in the code. Signed-off-by: Marcel Röthke <marcel@roethke.info> --- rerere.c | 3 +++ 1 file changed, 3 insertions(+)