Skip to content

Add process.dlopenFlags #12794

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.

Sign up for GitHub

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

Closed
ezequielgarcia wants to merge 2 commits into from
Closed

Add process.dlopenFlags #12794

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ec4386
constants: add dlopen flags
ezequielgarcia Apr 29, 2017
3ca00e7
src: add support to pass flags to dlopen
ezequielgarcia Jun 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
constants: add dlopen flags 
Let's add constants for dlopen flags, which are needed
for dlopen's flag passing, implemented in next commit.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
  • Loading branch information
@ezequielgarcia
ezequielgarcia committed Sep 8, 2017
commit 6ec4386776bf23b15043f9ab944cf0cc66e28e8f
37 changes: 37 additions & 0 deletions doc/api/os.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1170,6 +1170,43 @@ The following error codes are specific to the Windows operating system:
</tr>
</table>

### dlopen Constants

If available on the operating system, the following constants
are exported in `os.constants.dlopen`. See dlopen(3) for detailed
Copy link
Copy Markdown
Contributor

@thefourtheye thefourtheye Sep 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Care to link http://man7.org/linux/man-pages/man3/dlopen.3.html?

Copy link
Copy Markdown
Contributor Author

@ezequielgarcia ezequielgarcia Sep 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it done automagically? I think we already discussed this.

Copy link
Copy Markdown
Contributor

@thefourtheye thefourtheye Sep 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad. Yes, tools/doc/html.js will take care of this.

information.

<table>
<tr>
<th>Constant</th>
<th>Description</th>
</tr>
<tr>
<td><code>RTLD_LAZY</code></td>
Copy link
Copy Markdown
Contributor

@mscdex mscdex May 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing descriptions for these constants?

Copy link
Copy Markdown
Contributor Author

@ezequielgarcia ezequielgarcia May 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the descriptions were too detailed and complex, and was expecting the reader could refer to the manual instead? It's a 1:1 mapping of the dlopen flags, so there's not any secret sauce.

Copy link
Copy Markdown
Member

@jasnell jasnell May 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the idea is to refer the user to a manual, then having these as links to that manual would be ideal :-) ... still, a very brief one sentence description would be preferred (I know we're fairly inconsistent with that in the docs and we need to get better at it)

Copy link
Copy Markdown
Contributor Author

@ezequielgarcia ezequielgarcia May 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mscdex @jasnell I've added a line with some description. I agree it looks better now.

<td>Perform lazy binding. Node.js sets this flag by default.</td>
</tr>
<tr>
<td><code>RTLD_NOW</code></td>
<td>Resolve all undefined symbols in the library before dlopen(3)
returns.</td>
</tr>
<tr>
<td><code>RTLD_GLOBAL</code></td>
<td>Symbols defined by the library will be made available for symbol
resolution of subsequently loaded libraries.</td>
</tr>
<tr>
<td><code>RTLD_LOCAL</code></td>
<td>The converse of RTLD_GLOBAL. This is the default behavior if neither
flag is specified.</td>
</tr>
<tr>
<td><code>RTLD_DEEPBIND</code></td>
<td>Make a self-contained library use its own symbols in preference to
symbols from previously loaded libraries.</td>
</tr>
</table>

### libuv Constants

<table>
Expand Down
1 change: 1 addition & 0 deletions lib/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
// Deprecation Code: DEP0008
const constants = process.binding('constants');
Object.assign(exports,
constants.os.dlopen,
Copy link
Copy Markdown
Member

@TimothyGu TimothyGu Sep 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should add things to something that's already deprecated.

Copy link
Copy Markdown
Contributor Author

@ezequielgarcia ezequielgarcia Sep 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has already been discussed, on this same PR-thread. I'll let you do the digging. (BTW: this is where mailing lists outpass github PRs: it's so much harder to dig discussion history).

Copy link
Copy Markdown
Member

@TimothyGu TimothyGu Sep 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, in that case SGTM. Sorry about not reading through the entire thread – though in my defense it is quite long.

constants.os.errno,
constants.os.signals,
constants.fs,
Expand Down
32 changes: 32 additions & 0 deletions src/node_constants.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@
# endif // !OPENSSL_NO_ENGINE
#endif

#if defined(__POSIX__)
#include <dlfcn.h>
#endif

namespace node {

using v8::Local;
Expand Down Expand Up @@ -1238,6 +1242,28 @@ void DefineZlibConstants(Local<Object> target) {
NODE_DEFINE_CONSTANT(target, Z_DEFAULT_LEVEL);
}

void DefineDLOpenConstants(Local<Object> target) {
#ifdef RTLD_LAZY
NODE_DEFINE_CONSTANT(target, RTLD_LAZY);
#endif

#ifdef RTLD_NOW
NODE_DEFINE_CONSTANT(target, RTLD_NOW);
#endif

#ifdef RTLD_GLOBAL
NODE_DEFINE_CONSTANT(target, RTLD_GLOBAL);
#endif

#ifdef RTLD_LOCAL
NODE_DEFINE_CONSTANT(target, RTLD_LOCAL);
#endif

#ifdef RTLD_DEEPBIND
NODE_DEFINE_CONSTANT(target, RTLD_DEEPBIND);
#endif
}

} // anonymous namespace

void DefineConstants(v8::Isolate* isolate, Local<Object> target) {
Expand Down Expand Up @@ -1267,18 +1293,24 @@ void DefineConstants(v8::Isolate* isolate, Local<Object> target) {
CHECK(zlib_constants->SetPrototype(env->context(),
Null(env->isolate())).FromJust());

Local<Object> dlopen_constants = Object::New(isolate);
CHECK(dlopen_constants->SetPrototype(env->context(),
Null(env->isolate())).FromJust());

DefineErrnoConstants(err_constants);
DefineWindowsErrorConstants(err_constants);
DefineSignalConstants(sig_constants);
DefineSystemConstants(fs_constants);
DefineOpenSSLConstants(crypto_constants);
DefineCryptoConstants(crypto_constants);
DefineZlibConstants(zlib_constants);
DefineDLOpenConstants(dlopen_constants);

// Define libuv constants.
NODE_DEFINE_CONSTANT(os_constants, UV_UDP_REUSEADDR);
NODE_DEFINE_CONSTANT(fs_constants, UV_FS_COPYFILE_EXCL);

os_constants->Set(OneByteString(isolate, "dlopen"), dlopen_constants);
os_constants->Set(OneByteString(isolate, "errno"), err_constants);
os_constants->Set(OneByteString(isolate, "signals"), sig_constants);
target->Set(OneByteString(isolate, "os"), os_constants);
Copy link
Copy Markdown
Member

@TimothyGu TimothyGu Sep 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alphabetical order

Copy link
Copy Markdown
Contributor Author

@ezequielgarcia ezequielgarcia Sep 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.

Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-binding-constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ assert.deepStrictEqual(
);

assert.deepStrictEqual(
Object.keys(constants.os).sort(), ['UV_UDP_REUSEADDR', 'errno', 'signals']
Object.keys(constants.os).sort(), ['UV_UDP_REUSEADDR', 'dlopen', 'errno',
'signals']
Copy link
Copy Markdown
Contributor

@thefourtheye thefourtheye Sep 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style Nit: The second array would look better in the second line I guess.

);

// Make sure all the constants objects don't inherit from Object.prototype
Expand All @@ -26,5 +27,5 @@ function test(obj) {

[
constants, constants.crypto, constants.fs, constants.os, constants.zlib,
constants.os.errno, constants.os.signals
constants.os.dlopen, constants.os.errno, constants.os.signals
].forEach(test);