-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
unix: fix OpenBSD build #2458
unix: fix OpenBSD build #2458
Conversation
7e0a701
to
e72b81e
Compare
src/unix/fs.c
Outdated
@@ -547,6 +547,8 @@ static int uv__fs_statfs(uv_fs_t* req) { | |||
|
|||
#if defined(__sun) || defined(__MVS__) | |||
stat_fs->f_type = 0; /* f_type is not supported. */ | |||
#elif defined(__OpenBSD__) | |||
stat_fs->f_type = (uint64_t)buf.f_fstypename; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems incorrect. According to http://man.openbsd.org/4.4BSD-Lite2/statfs.2, there is a f_type
field on OpenBSD. Furthermore, f_fstypename
is a char array, and it's being assigned to a uint64_t
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about the man page here it points to 4.4BSD-Lite 2 system but if I point to the actual OpenBSD 6.5
http://man.openbsd.org/OpenBSD-6.5/statfs.2
There is none I did on purpose so the developer can cast it back to string afterwards but I can disable the field for OpenBSD if you disagree no worries :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah you're right about that. Sorry.
In that case, I think we should just set it to zero (meaning __OpenBSD__
can just be part of the previous #if
).
e72b81e
to
31ff36a
Compare
Mainly disabling source membership for udp feature, unsupported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, assuming that you're running these changes locally 😄.
It would be nice to get these in the CI.
@cjihrig |
Ah yes I have few virtual or real BSD here and there ;-) |
Mainly disabling source membership for udp feature, unsupported. PR-URL: #2458 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Saúl Ibarra Corretgé <s@saghul.net>
Landed in 59146f2, cheers! |
Mainly disabling source membership for udp feature, unsupported.