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.

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

#602 added unistd::mkfifo #774

Merged
merged 1 commit into from
Oct 11, 2017
Merged

#602 added unistd::mkfifo #774

merged 1 commit into from
Oct 11, 2017

Conversation

jpastuszek
Copy link
Contributor

Since libc has mkfifo already I have just copied mkdir and adapted to mkfifo. I have tested that on MacOS only.

src/unistd.rs Outdated
/// - the path name is too long (longer than `PATH_MAX`, usually 4096 on linux, 1024 on OS X)
///
/// For a full list consult
/// [man mkfifo(3)](http://man7.org/linux/man-pages/man3/mkfifo.3.html)
Copy link
Member

Choose a reason for hiding this comment

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

Odd that you apparently develop on OSX but reference Linux man pages. However, nix is cross-platform, so the best man pages to use are opengroup's. Could you please change the link to this?
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/mkfifo.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well not that odd considering all other man page links in that file are pointing to http://man7.org/linux.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we've been slowly moving them as we go, but we haven't mass-converted.

@asomers
Copy link
Member

asomers commented Oct 7, 2017

Could you also please add an entry to the CHANGELOG?

@Mic92
Copy link
Contributor

Mic92 commented Oct 8, 2017

you can probably adapt the tests from the code I made here: https://github.com/nix-rust/nix/pull/751/files#diff-899c5a4ff7ee773df222af8874b4761eR115

@jpastuszek
Copy link
Contributor Author

Is there anything else needed for this PR?

@asomers
Copy link
Member

asomers commented Oct 10, 2017

Just a squash, then I'll merge it.

@jpastuszek
Copy link
Contributor Author

Squashed. Thank you.

@asomers
Copy link
Member

asomers commented Oct 11, 2017

bors r+

bors bot added a commit that referenced this pull request Oct 11, 2017
774: #602 added unistd::mkfifo r=asomers a=jpastuszek

Since libc has mkfifo already I have just copied mkdir and adapted to mkfifo. I have tested that on MacOS only.
@bors
Copy link
Contributor

bors bot commented Oct 11, 2017

@bors bors bot merged commit 08c41c8 into nix-rust:master Oct 11, 2017
@asomers
Copy link
Member

asomers commented Oct 13, 2017

Sorry, but I just noticed one more problem: mkfifo belongs in src/sys/stat.rs, not src/unistd.rs, because its C prototype is located in sys/stat.h.

@asomers
Copy link
Member

asomers commented Oct 13, 2017

Damn my out-of-date browser windows! I guess this one slipped by.

@jpastuszek
Copy link
Contributor Author

Sorry, my bad. I should have read the comment on the top of the file.
My reasoning was that mkfifo is like mkdir or other kind of function that crates objects in the file system so I just placed it beside it...

You may also want to consider moving mkdir so others won't make this mistake in the future:

NAME
     mkdir, mkdirat -- make a directory file

SYNOPSIS
     #include <sys/stat.h>

     int
     mkdir(const char *path, mode_t mode);
NAME
     mkfifo -- make a fifo file

SYNOPSIS
     #include <sys/types.h>
     #include <sys/stat.h>

     int
     mkfifo(const char *path, mode_t mode);

From http://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/stat.h.html:

The following shall be declared as functions and may also be defined as macros. Function prototypes shall be provided.

int    chmod(const char *, mode_t);
int    fchmod(int, mode_t);
int    fstat(int, struct stat *);
int    lstat(const char *restrict, struct stat *restrict);
int    mkdir(const char *, mode_t);
int    mkfifo(const char *, mode_t);
[XSI][Option Start]
int    mknod(const char *, mode_t, dev_t);
[Option End]
int    stat(const char *restrict, struct stat *restrict);
mode_t umask(mode_t);

And indeed the http://pubs.opengroup.org/onlinepubs/7908799/xsh/unistd.h.html does not list mkfifo or mkdir.

Do you want me to do something about this?

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.

3 participants