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

[API] Use std::vector<uint8_t> instead of std:string #16

Open
da2ce7 opened this issue Jun 1, 2016 · 8 comments
Open

[API] Use std::vector<uint8_t> instead of std:string #16

da2ce7 opened this issue Jun 1, 2016 · 8 comments

Comments

@da2ce7
Copy link

da2ce7 commented Jun 1, 2016

When looking through the design of sodiumpp, I'm curious of the choice of std:string.

I would have thought that the more natural format for binary data (or UTF-8 encoded text), was std::vector<uint8_t>.

Was/Is is there a special reason for the choice of std::string ?

@tigusoft-vm
Copy link

tigusoft-vm commented Jun 1, 2016

By the way,

we have some modifications of the code including better support for locking secrets in memory e.g. "locked_string" - that will be offered as PR in some time in future
https://github.com/tigusoft-vm/sodiumpp/tree/tigusoft/sodiumpp

if sodiumpp would change to vector<uint8_t> then we can easily follow that in our small patchset as well.

@da2ce7 da2ce7 changed the title [API] Use std:vector<unint8_t> instead of std:string [API] Use std::vector<unint8_t> instead of std:string Jun 1, 2016
@da2ce7 da2ce7 changed the title [API] Use std::vector<unint8_t> instead of std:string [API] Use std::vector<uint8_t> instead of std:string Jun 1, 2016
@rubendv
Copy link
Owner

rubendv commented Jun 1, 2016

std::string was already used by the original NaCl C++ API, and I'd like to keep it that way for the low level API. For the high level API std::vector<uint8_t> is fine for me, but you would have to convert between the two when calling the low level API, which may get messy. I do think that using std::string will be more convenient for people to use, as you'll probably have your data as an std::string anyway when you're reading files.

@da2ce7
Copy link
Author

da2ce7 commented Aug 5, 2016

When a programmer looks at an API and sees std::string, he/she expects uft8 (or other text) encoded data.
When a programmer looks at an API and sees std::vector<unit8_t>, he/she expects binary data.

When designing API's they should be intuitive for those who use them.

@rubendv
Copy link
Owner

rubendv commented Aug 5, 2016

I agree that std::vector<uint8_t> is a better fit, but that would mean breaking with the NaCl C++ api: (e.g. https://nacl.cr.yp.to/box.html). I suppose we could overload them and implement both for the low level API?

@berkus
Copy link
Contributor

berkus commented Jun 25, 2017

std::string is a binary container, so there's no need really.

programmer expecting utf8 in a std::string should better go read the stdlib documentation.

@berkus
Copy link
Contributor

berkus commented Jul 1, 2017

It could, but if you're concerned with cleansing() the container it will work correctly regardless of SSO.

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

5 participants
@berkus @da2ce7 @rubendv @tigusoft-vm and others