Feature/fix freebsd build #332
Conversation
clang++ on FreeBSD was blaming v8 for using invalid casts from nullptr:
reinterpret_cast from 'nullptr_t' to '...' is not allowed
Replace casts with NULL, or NULL with 0 where applicable.
fix nodejs#324
FreeBSD does not support V4MAPPED.
| @@ -1266,8 +1266,14 @@ static void Initialize(Handle<Object> target, | |||
| Integer::New(env->isolate(), AF_UNSPEC)); | |||
| target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "AI_ADDRCONFIG"), | |||
| Integer::New(env->isolate(), AI_ADDRCONFIG)); | |||
| #ifdef __FreeBSD__ | |||
There was a problem hiding this comment.
Maybe #ifdef on AI_V4MAPPED? It's probably an issue on DragonFlyBSD as well, maybe the other BSDs too.
There was a problem hiding this comment.
The flag itself is present, unfortunately.
There was a problem hiding this comment.
I remember that it was broken in FreeBSD 8 but I would be sad and surprised if it's still broken in FreeBSD 10. Could it be that the tests fail because we allow V4MAPPED with IPv4 queries? Does the patch below fix the test?
diff --git a/lib/dns.js b/lib/dns.js
index 0a6e84a..74af0b2 100644
--- a/lib/dns.js
+++ b/lib/dns.js
@@ -113,6 +113,9 @@ exports.lookup = function lookup(hostname, options, callback) {
if (family !== 0 && family !== 4 && family !== 6)
throw new TypeError('invalid argument: family must be 4 or 6');
+ if (family !== 6 && (hints & exports.V4MAPPED))
+ throw new TypeError('V4MAPPED only supported for IPv6 queries');
+
callback = makeAsync(callback);
if (!hostname) {
diff --git a/test/parallel/test-dns.js b/test/parallel/test-dns.js
index 1de9bbf..6da04be 100644
--- a/test/parallel/test-dns.js
+++ b/test/parallel/test-dns.js
@@ -135,12 +135,14 @@ assert.doesNotThrow(function() {
assert.doesNotThrow(function() {
dns.lookup('www.google.com', {
+ family: 6,
hints: dns.V4MAPPED
}, noop);
});
assert.doesNotThrow(function() {
dns.lookup('www.google.com', {
+ family: 6,
hints: dns.ADDRCONFIG | dns.V4MAPPED
}, noop);
});There was a problem hiding this comment.
Nope, it won't help. The source of the problem is here: https://github.com/iojs/io.js/blob/v1.x/lib/net.js#L894-L900
There was a problem hiding this comment.
That looks plain wrong to me, it must have worked purely by accident on platforms that are a little more forgiving. I suggest dropping this change and filing an issue for the V4MAPPED bug.
There was a problem hiding this comment.
Well, the problem is that server.listen() does not work on FreeBSD. May be we could land it this way and open the issue?
|
/cc @jbergstroem |
FreeBSD does not return EISDIR when reading "/".
indutny
force-pushed
the
feature/fix-freebsd-build
branch
from
8b07fa6
to
4e458d7
Compare
| @@ -106,6 +106,9 @@ exports.lookup = function lookup(hostname, options, callback) { | |||
| hints !== (exports.ADDRCONFIG | exports.V4MAPPED)) { | |||
| throw new TypeError('invalid argument: hints must use valid flags'); | |||
| } | |||
|
|
|||
| if (process.platform === 'freebsd' || family !== 6) | |||
There was a problem hiding this comment.
Please add a FIXME comment explaining why this hack is currently needed.
There was a problem hiding this comment.
Oh, and I might have pasted the wrong code on IRC; I think that should be &&, not ||. Mea culpa!
|
The V8 patch should be upstreamed because V8 master seems to be affected as well. @indutny You want to do that? Or maybe @jbergstroem can shepherd it? |
|
@bnoordhuis I'll figure it out |
|
@bnoordhuis please check again, pushed the fix. |
| @@ -106,6 +106,11 @@ exports.lookup = function lookup(hostname, options, callback) { | |||
| hints !== (exports.ADDRCONFIG | exports.V4MAPPED)) { | |||
| throw new TypeError('invalid argument: hints must use valid flags'); | |||
| } | |||
|
|
|||
| // FIXME(indutny): V4MAPPED on FreeBSD results in EAI_BADFLAGS, because | |||
| // the kernel does not support it | |||
There was a problem hiding this comment.
Close enough, I guess. (It's libc, not the kernel. Details...)
|
LGTM. Please file an issue for the V4MAPPED thing. |
FreeBSD does not support V4MAPPED. PR-URL: #332 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: #332 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: #332 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
FreeBSD does not return EISDIR when reading "/". PR-URL: #332 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
As always, AEDT-timezone kicks my review butt. Thanks for this. |
clang++ on FreeBSD was blaming v8 for using invalid casts from nullptr:
reinterpret_cast from 'nullptr_t' to '...' is not allowed
Replace casts with NULL, or NULL with 0 where applicable.
Fixes: nodejs#324
PR-URL: nodejs#332
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
n/t
cc @bnoordhuis