patch_parse: do not depend on parsed buffer's lifetime#5158
Conversation
When parsing a patch from a buffer, we let the patch lines point into the original buffer. While this is efficient use of resources, this also ties the lifetime of the parsed patch to the parsed buffer. As this behaviour is not documented anywhere in our API it is very surprising to its users. Untie the lifetime by duplicating the lines into the parsed patch. Add a test that verifies that lifetimes are indeed independent of each other.
|
Hmm, so a very quick skim indicates that |
Hmm, after a slightly less quick skim, I think that my very quick skim was too quick for me to understand the lifecycle of the buffer and usage. I'm going to have to 👀 this over the weekend to grok it in fullness. |
|
You can definitely trigger this error by using our public APIs, only. See the "git diff --no-index" example in #5125, which does trigger this error by free'ing the file contents before printing the patch. |
|
I think I've just encountered this myself after parsing a patch BEFORE AFTER (contains garbage at 1st diff line) |
|
Yeah, definitely looks like the same issue. It's really easy to trigger, unfortunately |
When parsing a patch from a buffer, we let the patch lines point into
the original buffer. While this is efficient use of resources, this also
ties the lifetime of the parsed patch to the parsed buffer. As this
behaviour is not documented anywhere in our API it is very surprising to
its users.
Untie the lifetime by duplicating the lines into the parsed patch. Add a
test that verifies that lifetimes are indeed independent of each other.
Noticed while testing #5125