Repository navigation
smalloc: extend user API - #905
trevnorris wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
shouldn't it be AllocDispose(Environment::GetCurrent(isolate), obj)?
There was a problem hiding this comment.
hehe. i'll add that to the test.
|
LGTM, if it works |
There was a problem hiding this comment.
Maybe move it to smalloc-internal.h?
There was a problem hiding this comment.
If so then I'd recommend doing that in a different PR, and doing with all other headers that do the same (e.g. node_buffer.h).
node::Environment isn't accessible to user APIs, so extend smalloc to also accept v8::Isolate. Fixes: 75adde0 "src: remove `node_isolate` from source"
ee61b42 to
ba10fd7
Compare
|
@vkurchatkin addressed comments. thanks. |
|
Landed in 26ebe98. |
node::Environment isn't accessible to user APIs, so extend smalloc to also accept v8::Isolate. Fixes: 75adde0 "src: remove `node_isolate` from source" PR-URL: nodejs/node#905 Reviewed-by: Fedor Indutny <fedor@indutny.com>
|
should this have the semver-minor tag? is it a feature add? |
|
@rvagg No. It's a fix to an unusable API. Check the "Fixes" in the commit message for the commit that broke it. This happened when |
node::Environment isn't accessible to user APIs, so extend smalloc to
also accept v8::Isolate.
Fixes: 75adde0 "src: remove
node_isolatefrom source"R=@bnoordhuis
R=@indutny