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

String length and allocation #6

Open
Sod-Almighty opened this issue Dec 1, 2018 · 5 comments
Open

String length and allocation #6

Sod-Almighty opened this issue Dec 1, 2018 · 5 comments

Comments

@Sod-Almighty
Copy link

When calling the readString functions, I have to magically know how big the string buffer needs to be. There appears to be no way to check this ahead of time.

I propose a variant of readString, thusly:

char* readNewString(Stream& stream, bool safely = true) {
 DataType dataFormat;
 getNextDataType(stream, dataFormat, safely);
 size_t outputSize;
 char* value = nullptr;
 switch(dataFormat) {
  case DataType::String5: {
   MSGPACK_SAFETY_FORMAT_CHECK(DataType::String8);
   stream.read();
   MSGPACK_SAFELY_RUN(readRaw(stream, outputSize, safely));
   value = new char[outputSize + 1];
   MSGPACK_SAFELY_RUN(readRaw(stream, value, outputSize, safely));
   value[outputSize] = '\0';
   return value;
  }
  /* ... */
 }
}
@elliotwoods
Copy link
Owner

Yes it's a good idea!
Could you possibly make a pull request?

Naming-wise, it might be nice to be readStringNew since it is a variant of readString... (so that it shows up in auto-complete along-side the others).

@Sod-Almighty
Copy link
Author

Issued pull request #7

Incidentally, is there a simple way to add a function that skips whatever the next value is?

@elliotwoods
Copy link
Owner

elliotwoods commented Dec 10, 2018

we don't have a generic skip value function.
So if the next value was a map of maps, then we'd want to skip the map of maps - so the skip code would need to recursively skip all sub-elements, correct?

@Sod-Almighty
Copy link
Author

Correct. Would you like me to have a go?

@elliotwoods
Copy link
Owner

sure! why not. that'd be great

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

No branches or pull requests

2 participants