This repository has been archived by the owner. It is now read-only.

Make possible to get Buffer from crypto calls #3989

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
3 participants
@langpavel

See #3986

This allows to call this to get buffer:

var crypto = require('crypto');
var hash = crypto.createHash('sha1');
hash.update('test');
var buf = hash.digest('buffer');
// buf now contains <SlowBuffer a9 4a 8f ... bb d3>

Tests needed, I can write them next week, so busy now...

src/node.cc
+
+ Buffer* buffer = Buffer::New(_buffer, len, EncodeBufferFree, NULL);
+ return scope.Close(Local<Object>::New(buffer->handle_));
+ }

This comment has been minimized.

Show comment Hide comment
@bnoordhuis

bnoordhuis Sep 9, 2012

Member

Simplify this to something like:

if (encoding == BUFFER) {
  Buffer* b = Buffer::New(len);
  memcpy(Buffer::Data(b), buf, len);
  return scope.Close(b->handle_);
}
@bnoordhuis

bnoordhuis Sep 9, 2012

Member

Simplify this to something like:

if (encoding == BUFFER) {
  Buffer* b = Buffer::New(len);
  memcpy(Buffer::Data(b), buf, len);
  return scope.Close(b->handle_);
}
@bnoordhuis

This comment has been minimized.

Show comment Hide comment
@bnoordhuis

bnoordhuis Sep 9, 2012

Member

I'm okay with the conceptual change. Can you add a few tests to test/simple/test-crypto.js?

Member

bnoordhuis commented Sep 9, 2012

I'm okay with the conceptual change. Can you add a few tests to test/simple/test-crypto.js?

src/node.cc
+ if (encoding == BUFFER) {
+ Buffer* buffer = Buffer::New(len);
+ memcpy(Buffer::Data(buffer), buf, len);
+ return scope.Close(buffer->handle_);

This comment has been minimized.

Show comment Hide comment
@langpavel

langpavel Sep 15, 2012

Ok, at this poin should be used Buffer::New(const char*, size_t)
@bnoordhuis can I merge with master branch? (diff can be unreadable at github)

@langpavel

langpavel Sep 15, 2012

Ok, at this poin should be used Buffer::New(const char*, size_t)
@bnoordhuis can I merge with master branch? (diff can be unreadable at github)

@bnoordhuis

This comment has been minimized.

Show comment Hide comment
@bnoordhuis

bnoordhuis Sep 17, 2012

Member

@indutny You had an opinion about this PR?

Member

bnoordhuis commented Sep 17, 2012

@indutny You had an opinion about this PR?

langpavel added some commits Sep 13, 2012

buffer: update constructor prototype
Change Buffer::New(char*, size_t) to Buffer::New(const char*, size_t).
@langpavel

This comment has been minimized.

Show comment Hide comment
@langpavel

langpavel Sep 17, 2012

Closed in favour of #4007

Closed in favour of #4007

@langpavel langpavel closed this Sep 17, 2012

@indutny

This comment has been minimized.

Show comment Hide comment
@indutny

indutny Sep 18, 2012

Member

@bnoordhuis yes I seen it, but after releasing first version of my patch. Sorry @langpavel, you did a good job.

Member

indutny commented Sep 18, 2012

@bnoordhuis yes I seen it, but after releasing first version of my patch. Sorry @langpavel, you did a good job.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.