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

object: Improve correctness #105

Merged
merged 1 commit into from
Nov 8, 2024
Merged

Conversation

javierhonduco
Copy link
Owner

object: Improve correctness
Due to the fact that object::File takes a reference to the mmap-ed
file, we needed to do some awful lifetime hacks to fit it in the API we
wanted. While getting rid of the raw pointer and drop impl, Ricardo
noticed that there was some lurking UB too, so this fixes it as well.

Co-authored-by: Ricardo Delfin [email protected]

Test Plan

CI

@javierhonduco javierhonduco force-pushed the object-correctness-improvements branch from 33390b0 to 1887e3f Compare November 4, 2024 15:41
Copy link
Collaborator

@rdelfin rdelfin left a comment

Choose a reason for hiding this comment

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

Co-authored-by: Ricardo Delfin [email protected]

Some truly slanderous statements

Just a couple comments

lightswitch-object/src/object.rs Outdated Show resolved Hide resolved
lightswitch-object/src/object.rs Outdated Show resolved Hide resolved
@javierhonduco javierhonduco force-pushed the object-correctness-improvements branch from 1887e3f to f294e2b Compare November 8, 2024 10:30
@javierhonduco javierhonduco requested a review from rdelfin November 8, 2024 10:30
Copy link
Collaborator

@rdelfin rdelfin left a comment

Choose a reason for hiding this comment

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

LGTM

lightswitch-object/src/object.rs Outdated Show resolved Hide resolved
Due to the fact that `object::File` takes a reference to the mmap-ed
file, we needed to do some awful lifetime hacks to fit it in the API we
wanted. While getting rid of the raw pointer and drop impl, Ricardo
noticed that there was some lurking UB too, so this fixes it as well.

Co-authored-by: Ricardo Delfin <[email protected]>

Test Plan
=========

CI
@javierhonduco javierhonduco force-pushed the object-correctness-improvements branch from f294e2b to 29d5970 Compare November 8, 2024 10:42
@javierhonduco javierhonduco merged commit e3e8e9c into main Nov 8, 2024
4 checks passed
@javierhonduco javierhonduco deleted the object-correctness-improvements branch November 25, 2024 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants