Skip to content

ext/posix: adding posix_mkfifoat/posix_mknodat. #13829

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

devnexen
Copy link
Member

The additional directory file descriptor argument allows to operate from a relative path perspective but those new calls can still work with absolute paths.

@devnexen devnexen force-pushed the posix_mkfifoat branch 5 times, most recently from b2346e1 to d0fcce3 Compare March 29, 2024 06:30
@devnexen devnexen marked this pull request as ready for review March 29, 2024 06:44
@devnexen devnexen requested a review from kocsismate as a code owner March 29, 2024 06:44
Copy link
Member

@bukka bukka left a comment

Choose a reason for hiding this comment

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

It would be good if you could provide some better test showing how it could be useful in PHP context.

Comment on lines +30 to +42
bool(false)
bool(false)
posix_mknodat(): Argument #4 ($major) cannot be 0 for the POSIX_S_IFCHR and POSIX_S_IFBLK modes
bool(false)
bool(false)
posix_mkfifoat(): Argument #1 ($file_descriptor) must be of type int|resource, stdClass given
Copy link
Member

Choose a reason for hiding this comment

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

this is pretty much just a failure test. Can you write some success test with appropriate skips?

#ifdef HAVE_MAKEDEV
php_dev = makedev(major, minor);
#else
zend_value_error("Cannot create a block or character device, creating a normal file instead");
Copy link
Member

Choose a reason for hiding this comment

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

This should be a warning if we want to continue the execution

The additional directory file descriptor argument allows to operate
from a relative path perspective but those new calls can still
work with absolute paths.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants