Skip to content
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

Jump to bottom

O_TMPFILE is includes O_DIRECTORY #501

Closed
wants to merge 2 commits into from
Closed

O_TMPFILE is includes O_DIRECTORY #501

wants to merge 2 commits into from

Conversation

TethysSvensson
Copy link
Contributor

@posborne
Copy link
Member

@idolf Since this code is Linux/Android specifc, I have no problem with the change. Android is still on a 3.10 kernel on many systems, but I'm fine with the value returning an EINVAL on older kernels (this doesn't change that behavior). Can you add a note on this to the CHANGELOG.md file?

@TethysSvensson
Copy link
Contributor Author

Changelog update.

As far as I can tell, the value of O_TMPFILE has always been defined as 0o20000000 | O_DIRECTORY, however I am just now noticing that the value of O_DIRECTORY is not architecture independent. 😧

I don't think that this fix will be sufficient.

@posborne
Copy link
Member

I don't think that this fix will be sufficient.

libc defines O_DIRECTORY in an architecture-specific way, so we should move to using/reusing that value as we do elsewhere in nix. We still want the enumerations we provide as part of the nix safety story, but we should use the actual value from libc.

The O_TMPFILE constant also really belongs in libc and not nix (obviously that hasn't been changes in this code as of yet at all and would require some contributions to libc). If you are up for it, I would love a volunteer to help make this happen.

@TethysSvensson TethysSvensson mentioned this pull request Jan 31, 2017
Merged
@TethysSvensson TethysSvensson deleted the fix-o-tmpfile branch January 31, 2017 20:13
homu added a commit that referenced this pull request Feb 23, 2017
@homu
Auto merge of #503 - Idolf:bitflags, r=kamalmarhubi
1fb5fe3 
Fix the style for bitflags!

Prefer `libc_bitflags!` over `bitflags!`. Prefer `libc::CONSTANTS` over
writing the constant manually.

This makes #501 unnecessary, since upstream now contains the `O_TMPFILE` constant.
homu added a commit that referenced this pull request Feb 23, 2017
@homu
Auto merge of #503 - Idolf:bitflags, r=kamalmarhubi
2bb718c 
Fix the style for bitflags!

Prefer `libc_bitflags!` over `bitflags!`. Prefer `libc::CONSTANTS` over
writing the constant manually.

This makes #501 unnecessary, since upstream now contains the `O_TMPFILE` constant.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@TethysSvensson @posborne