Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix lifetimes of outputs for Patch #1141

Merged
merged 1 commit into from
Mar 12, 2025
Merged

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Mar 12, 2025

This fixes the lifetimes for the functions that return data from a Patch. From what I can tell, these are returning data owned by the Patch object itself. Previously they had lifetimes tied to the values that created the Patch, but that allows these outputs to outlive the patch, allowing a use-after-free.

Note that there was a previous fix here in #523, but I think that was mistake to change these accessor functions. The Patch itself still has a lifetime tied to its inputs introduced in #523.

Fixes #1135

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@rustbot rustbot added the S-waiting-on-review Status: Waiting on review label Mar 12, 2025
@ehuss ehuss mentioned this pull request Mar 12, 2025
@ehuss
Copy link
Contributor Author

ehuss commented Mar 12, 2025

@weihanglo Would you be able to review this? It's a bit difficult at times to understand the ownership requirements, but I think this is correct.

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking with this libgit2 code snippet, it looks correct. At this it should be there when patch is still valid. Thanks!

@weihanglo weihanglo added this pull request to the merge queue Mar 12, 2025
Merged via the queue into rust-lang:master with commit e8c9126 Mar 12, 2025
7 checks passed
naseschwarz pushed a commit to naseschwarz/gitui that referenced this pull request Mar 18, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
git2-rs changed the lifetime of the result of Patch::hunk() to reference
the patch struct:
rust-lang/git2-rs#1141

Thus, returning a patch struct and a reference into it is flagged by the
borrow checker upon move when returning.

This patch avoids returning both alltogether by separating retrieving
and retrieving the patch.
extrawurst pushed a commit to gitui-org/gitui that referenced this pull request Mar 18, 2025
* Bump git2 from 0.20.0 to 0.20.1

Bumps [git2](https://github.com/rust-lang/git2-rs) from 0.20.0 to 0.20.1.
- [Changelog](https://github.com/rust-lang/git2-rs/blob/master/CHANGELOG.md)
- [Commits](rust-lang/git2-rs@git2-0.20.0...git2-0.20.1)

---
updated-dependencies:
- dependency-name: git2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* Avoid passing reference into struct

git2-rs changed the lifetime of the result of Patch::hunk() to reference
the patch struct:
rust-lang/git2-rs#1141

Thus, returning a patch struct and a reference into it is flagged by the
borrow checker upon move when returning.

This patch avoids returning both alltogether by separating retrieving
and retrieving the patch.

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Naseschwarz <naseschwarz@0x53a.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Waiting on review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DiffLine mangled memory
3 participants