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

Return expected length substring rather than throw an exception when reading strings #500

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Himmelt
Copy link
Contributor

@Himmelt Himmelt commented Aug 15, 2023

Return expected length substring rather than throw an exception when reading strings

@mycroes
Copy link
Member

mycroes commented Aug 16, 2023

HI @Himmelt,

First off, thanks for your contribution. I'm not sure what to think about this change. While the remark with regards to reading a single string sounds quite fair, this doesn't work in different contexts. When someone attempts to read a string as part of a struct, the incorrect length will lead to a whole new set of errors that might go on undetected.

I'd prefer to have a separate bool TryGetString(byte[] input, out string result, out int size) instead, that would return true iff the full string can be read from input and sets result and size to their appropriate values, or false when input doesn't contain enough bytes, setting result to the partial decoded string and still assigning size to the reserved size for the string.

If you want to implement the above functionality feel free to do that here or in a new PR, I prefer to have the current commits dropped entirely instead of reverted though.

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