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

StringBuffer.GetSize is the byte count, not length #744

Closed
heaths opened this issue Sep 21, 2016 · 1 comment
Closed

StringBuffer.GetSize is the byte count, not length #744

heaths opened this issue Sep 21, 2016 · 1 comment

Comments

@heaths
Copy link

heaths commented Sep 21, 2016

While the name GetSize I arguably obvious that its a byte count, the fact it's on a StringBuffer can make it's use questionable. For example, all examples of serializing a JSON graph to a string do something like so:

string content(buffer.GetString(), buffer.GetSize());

But when typedef'ing a StringBuffer with UTF16<>, the following code can cause access violations.

wstring content(buffer.Getstring(), buffer.GetSize());

Instead, we need to divide the character byte length:

wstring content(buffer.Getstring(), buffer.GetSize() / sizeof(WStringBuffer::Ch));

Since the Ch typedef is known, and thus the byte size, adding a GetLength() and using that in examples to avoid confusion would be advisable.

@miloyip miloyip added the bug label Sep 22, 2016
heaths added a commit to heaths/rapidjson that referenced this issue Sep 22, 2016
miloyip added a commit that referenced this issue Sep 22, 2016
@miloyip miloyip added enhancement and removed bug labels Sep 22, 2016
@heaths
Copy link
Author

heaths commented Sep 22, 2016

That works too. I had a change I was about to send a PR for, but thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants