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

In node_buffer.h, why do all the calls accept (and return) char * instead of unsigned char *? #3162

Closed
sgerace opened this issue Oct 2, 2015 · 6 comments
Labels
buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. question Issues that look for answers.

Comments

@sgerace
Copy link

sgerace commented Oct 2, 2015

I'm writing a node addon and I was just curious if there was a reason why all of the call in node::Buffer accept and return char * instead of the (arguably) more representative unsigned char *. I'm analyzing buffers as they are passed into my function, so it is certainly trivial for me to do:

unsigned char *data = (unsigned char *)node::Buffer::Data(args[0]);

I was just wondering if there was a specific reason why char * was chosen since buffers (by definition) are intended to contain binary data.

@mscdex mscdex added question Issues that look for answers. buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. labels Oct 2, 2015
@mscdex
Copy link
Contributor

mscdex commented Oct 3, 2015

/cc @trevnorris

@bnoordhuis
Copy link
Member

It's pretty much an accident of history. I'd have picked uint8_t* myself.

@trevnorris
Copy link
Contributor

Probably has something to do with the old v8 api working with char*. But it is for legacy reasons.

@trevnorris
Copy link
Contributor

@bnoordhuis Would we consider changing the native API again, or is this something we'll be leaving alone? char* may have been chosen to work most easily with the V8 API at the time, but that isn't so much the case now.

Personally I'd say we leave it as is unless there's a compelling reason to change it.

@bnoordhuis
Copy link
Member

I'm -0 on changing it now. It would cause a lot of breakage in the ecosystem for comparatively little benefit.

@smartmouse
Copy link

totally agree, pls do not break things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. question Issues that look for answers.
Projects
None yet
Development

No branches or pull requests

5 participants