Skip to content

Implement mkfifoat #1133

Merged
bors[bot] merged 3 commits intonix-rust:masterfrom
zmlcc:fea-mkfifoat
Oct 7, 2019
Merged

Implement mkfifoat #1133
bors[bot] merged 3 commits intonix-rust:masterfrom
zmlcc:fea-mkfifoat

Conversation

@zmlcc
Copy link
Copy Markdown
Contributor

@zmlcc zmlcc commented Sep 30, 2019

This adds the mkfifoat function, which is part of POSIX https://pubs.opengroup.org/onlinepubs/9699919799/functions/mkfifoat.html

test cases are copied from mkfifo

@zmlcc zmlcc mentioned this pull request Sep 30, 2019
14 tasks
Copy link
Copy Markdown
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

Good start, but don't forget to add a CHANGELOG.

Comment thread test/test_unistd.rs Outdated

let stats = stat::stat(&mkfifoat_fifo).unwrap();
let typ = stat::SFlag::from_bits_truncate(stats.st_mode);
assert!(typ == SFlag::S_IFIFO);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be assert_eq instead, here and elsewhere.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Comment thread test/test_unistd.rs
let typ = stat::SFlag::from_bits_truncate(stats.st_mode);
assert!(typ == SFlag::S_IFIFO);


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Only one blank line, please.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Comment thread test/test_unistd.rs
assert!(typ == SFlag::S_IFIFO);


let dirfd = open(tempdir.path(), OFlag::empty(), Mode::empty()).unwrap();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This part should really be a separate test case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Comment thread test/test_unistd.rs
// mkfifoat should fail if a directory is given
assert!(mkfifoat(None, &env::temp_dir(), Mode::S_IRUSR).is_err());

let tempdir = tempfile::tempdir().unwrap();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This part should be a separate test case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

ignore mkfifoat in OSX and andriod
@zmlcc
Copy link
Copy Markdown
Contributor Author

zmlcc commented Sep 30, 2019

@asomers All comments are fixed, please review
Also have a look at #1131 and #1134, thanks

Copy link
Copy Markdown
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

LGTM.

bors r+

bors Bot added a commit that referenced this pull request Oct 7, 2019
1133: Implement mkfifoat  r=asomers a=zmlcc

This adds the `mkfifoat ` function, which is part of POSIX [https://pubs.opengroup.org/onlinepubs/9699919799/functions/mkfifoat.html](https://pubs.opengroup.org/onlinepubs/9699919799/functions/mkfifoat.html)

test cases are copied from `mkfifo`


Co-authored-by: Zhang Miaolei <zmlcc@outlook.com>
@bors
Copy link
Copy Markdown
Contributor

bors Bot commented Oct 7, 2019

Build succeeded

@bors bors Bot merged commit 4b23433 into nix-rust:master Oct 7, 2019
@zmlcc zmlcc deleted the fea-mkfifoat branch October 11, 2019 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants