Skip to content

Conversation

ids1024
Copy link
Contributor

@ids1024 ids1024 commented Jul 4, 2017

No description provided.

@rust-highfive
Copy link
Contributor

r? @BurntSushi

(rust_highfive has picked a reviewer for you, use r? to override)

@aidanhs
Copy link
Contributor

aidanhs commented Jul 5, 2017

Thanks for the PR @ids1024, we'll make sure @BurntSushi or another reviewer gets to this soon!

@aidanhs aidanhs added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 5, 2017
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW this should probably also specify O_RDONLY to obey posix (since redox seems to be posixish).

(it's an interesting dilemma whether redox should reject that syscall as-is - it's technically invalid, but linux would accept it because O_RDONLY is defined as 0, so is always implicit. In redox it's defined as 1, so there's a choice between correctness and Linux program compatibility)

Copy link
Contributor

@jackpot51 jackpot51 Jul 8, 2017

Choose a reason for hiding this comment

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

False. O_STAT in Redox allows a file handle to be opened on a file without read permission, if the user has the ability to read the directory. This file handle can only be used for things like fstat.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see O_STAT is not posix, I guess that justifies my 'ish' in 'posixish'. Thanks for the clarification.

@arielb1
Copy link
Contributor

arielb1 commented Jul 11, 2017

Hi @BurntSushi - are you going to be reviewing this? Should someone else be reviewing it? Just trying to keep it going.

@BurntSushi
Copy link
Member

@arielb1 Ah thanks, I missed this.

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 11, 2017

📌 Commit 4e26908 has been approved by BurntSushi

@bors
Copy link
Collaborator

bors commented Jul 11, 2017

🔒 Merge conflict

@aturon
Copy link
Contributor

aturon commented Jul 11, 2017

@bors: r=burntsushi

@bors
Copy link
Collaborator

bors commented Jul 11, 2017

📌 Commit 51260f4 has been approved by burntsushi

@bors
Copy link
Collaborator

bors commented Jul 12, 2017

⌛ Testing commit 51260f4 with merge cd71ea7...

bors added a commit that referenced this pull request Jul 12, 2017
Redox: Use O_NOFOLLOW for lstat()
@bors
Copy link
Collaborator

bors commented Jul 12, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: burntsushi
Pushing cd71ea7 to master...

@bors bors merged commit 51260f4 into rust-lang:master Jul 12, 2017
@ids1024 ids1024 deleted the nofollow2 branch July 12, 2017 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants