fs: use preadv on Linux if available #1156

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
@mscdex
Contributor

mscdex commented Feb 26, 2014

No description provided.

@Nodejs-Jenkins

This comment has been minimized.

Show comment Hide comment
@Nodejs-Jenkins

Nodejs-Jenkins Feb 26, 2014

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

The following commiters were not found in the CLA:

  • Brian White

You can fix all these things without opening another issue.

Please see CONTRIBUTING.md for more information

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

The following commiters were not found in the CLA:

  • Brian White

You can fix all these things without opening another issue.

Please see CONTRIBUTING.md for more information

@saghul

This comment has been minimized.

Show comment Hide comment
@saghul

saghul Feb 26, 2014

Contributor

Looks like build is failing on non-Linux systems:

../src/unix/fs.c:56:42: error: token is not a valid binary operator in a preprocessor subexpression
     LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30) &&                      \
                           ~~~~~~~~~~~~~~^

Maybe we need another set of "if"s just for linux?

Contributor

saghul commented Feb 26, 2014

Looks like build is failing on non-Linux systems:

../src/unix/fs.c:56:42: error: token is not a valid binary operator in a preprocessor subexpression
     LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30) &&                      \
                           ~~~~~~~~~~~~~~^

Maybe we need another set of "if"s just for linux?

@indutny

This comment has been minimized.

Show comment Hide comment
@indutny

indutny Feb 26, 2014

Contributor

I bet it should be inside separate ifdef.

Contributor

indutny commented Feb 26, 2014

I bet it should be inside separate ifdef.

@mscdex

This comment has been minimized.

Show comment Hide comment
@mscdex

mscdex Feb 26, 2014

Contributor

changed :-)

Contributor

mscdex commented Feb 26, 2014

changed :-)

@saghul

This comment has been minimized.

Show comment Hide comment
@saghul

saghul Feb 26, 2014

Contributor

Looks a bit convoluted :-) How about setting it to 1 or 0 for the BSDs and then doing the Linux check so that we only set it to 1 if all conditions are met else it will remain as 0?

Contributor

saghul commented Feb 26, 2014

Looks a bit convoluted :-) How about setting it to 1 or 0 for the BSDs and then doing the Linux check so that we only set it to 1 if all conditions are met else it will remain as 0?

@mscdex

This comment has been minimized.

Show comment Hide comment
@mscdex

mscdex Feb 26, 2014

Contributor

If we do that, gcc warns about redefining. Is that ok?

Contributor

mscdex commented Feb 26, 2014

If we do that, gcc warns about redefining. Is that ok?

@saghul

This comment has been minimized.

Show comment Hide comment
@saghul

saghul Feb 26, 2014

Contributor

Oh, forget it then :-) LGTM.

Contributor

saghul commented Feb 26, 2014

Oh, forget it then :-) LGTM.

@saghul

This comment has been minimized.

Show comment Hide comment
@saghul

saghul Feb 26, 2014

Contributor

Thanks a lot! Landed in 269ff0b

Contributor

saghul commented Feb 26, 2014

Thanks a lot! Landed in 269ff0b

@saghul saghul closed this Feb 26, 2014

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