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: Add zero and negative byte length checks to ensure #110

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

414owen
Copy link

@414owen 414owen commented Mar 14, 2024

Summary

ensure is a publically exported function, which has some odd behaviour when given a byte length of zero or less.

It raises an error when the value is zero, but happily returns a value when the byte length is negative.

I've switched these two behaviours around.

Current behaviour:

λ> runGet (ensure (-1)) "0123"
Right "0123"
λ> runGet (isolate 0 getWord8) "0123"
Left "too few bytes\nFrom:\tdemandInput\n\n"

New behaviour:

λ> runGet (ensure (-1)) "0123"
Left "Failed reading: Attempted to ensure negative amount of bytes\nEmpty call stack\n"
λ> runGet (ensure 0) "0123"
Right ""

`ensure` is a publically exported function, which has some odd behaviour
when given a byte length of zero or less.

It raises an error when the value is zero, but happily returns a value
when the byte length is negative.

I've switched these two behaviours around.

```
λ> runGet (ensure (-1)) "0123"
Right "0123"
λ> runGet (isolate 0 getWord8) "0123"
Left "too few bytes\nFrom:\tdemandInput\n\n"
```

```
λ> runGet (ensure (-1)) "0123"
Left "Failed reading: Attempted to ensure negative amount of bytes\nEmpty call stack\n"
λ> runGet (ensure 0) "0123"
Right ""
```
@414owen
Copy link
Author

414owen commented Mar 14, 2024

Ugh... When one of the functions in Get.hs calls ensure 0, it ends up putting the empty bytestring...
We'd also need guards around those functions... which maybe isn't the worst idea...
ensure seems like an odd thing to expose to the user, since it doesn't update the read count, and doesn't have peek in the name...

This started out as a four line change...

This adds an internal variant, `ensure'`, which is used to avoid
checking for `0` and `<0` more than once in a given code path.
@414owen 414owen force-pushed the os/protect-ensure-values branch from e208da5 to 024bba7 Compare March 15, 2024 09:13
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.

1 participant