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.

Sign up for GitHub

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

Jump to bottom

refactor out duplicate code #41

Merged
merged 3 commits into from
Apr 7, 2022
Merged

refactor out duplicate code #41

merged 3 commits into from
Apr 7, 2022

Conversation

jarkkojs
Copy link
Contributor

@jarkkojs jarkkojs commented Apr 2, 2022

No description provided.

@jarkkojs jarkkojs mentioned this pull request Apr 2, 2022
Closed
@jarkkojs
Copy link
Contributor Author

jarkkojs commented Apr 2, 2022

@npmccallum I did these changes to factor out any possibilities of mmledger failing as I'm debugging SGX2. There's also added self-consistency checks in addition to minimizing the code paths.

@jarkkojs jarkkojs mentioned this pull request Apr 2, 2022
Merged
@jarkkojs
Copy link
Contributor Author

jarkkojs commented Apr 3, 2022 •
edited
Loading

In SGX we must have mmledger to detect overlaps because it will be a long time, if ever, we support them. Such is issue needs to be detected when it happens.

By dividing the problem into nicely encapsulated components, it's easy to make the call to delete() opt-in without messing the code. It would also permit make merging optional but for that at least SGX does not have immediate need.

haraldh
haraldh previously approved these changes Apr 6, 2022
View reviewed changes
@npmccallum npmccallum enabled auto-merge (rebase) April 6, 2022 14:37
npmccallum
npmccallum requested changes Apr 6, 2022
View reviewed changes
src/lib.rs Outdated Show resolved Hide resolved
@jarkkojs
refactor(tests): create initial data within tests.
4e9a85b 
Re-implement existing test cases with an input array of data, and
provide visually more appealing feedback when a test fails so that
analysis will be quicker.

Signed-off-by: Jarkko Sakkinen <jarkko@profian.com>
@jarkkojs
refactor: merge duplicate code
cec9e5b 
Intersections can be sorted out by `unmap()`, so use that instead of having
two duplicate implementations for this, and encapsulate `merge()` as a
separate pass. This reduces the surface for possible bugs, and improves
test coverage at the same time, given that all `map()` tests now test also
`unmap()` code.

Closes: #40
Signed-off-by: Jarkko Sakkinen <jarkko@profian.com>
@jarkkojs
refactor: length -> tail
373a6d4 
Signed-off-by: Jarkko Sakkinen <jarkko@profian.com>
@jarkkojs
Copy link
Contributor Author

jarkkojs commented Apr 6, 2022

@npmccallum please review

haraldh
haraldh previously approved these changes Apr 7, 2022
View reviewed changes
@jarkkojs
Copy link
Contributor Author

jarkkojs commented Apr 7, 2022

@npmccallum mmap PR is not anymore dependent on this but please approve this when you can because it is needed for continuation.

@jarkkojs
Copy link
Contributor Author

jarkkojs commented Apr 7, 2022

Added snapshot of the Heap to this PR. It can refined from this point forward.

@jarkkojs
Copy link
Contributor Author

jarkkojs commented Apr 7, 2022

moved heap to a separate branch which i upstream later on

@jarkkojs jarkkojs requested a review from haraldh April 7, 2022 11:49
@haraldh haraldh disabled auto-merge April 7, 2022 14:40
@haraldh haraldh merged commit 6959aa9 into enarx:main Apr 7, 2022
@jarkkojs jarkkojs deleted the refactor/duplicate branch April 11, 2022 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@npmccallum npmccallum Awaiting requested review from npmccallum

@haraldh haraldh Awaiting requested review from haraldh

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jarkkojs @npmccallum @haraldh