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

add 'e' flag for fopen() to enable CLOEXEC #1546

Closed
wants to merge 1 commit into from

Conversation

7 participants
@eelf
Copy link
Contributor

commented Oct 2, 2015

introduces a new mode character 'e' for fopen to add O_CLOEXEC flag
useful for preventing leaking of file descriptors to child processes

@laruence

This comment has been minimized.

Copy link
Member

commented Oct 14, 2015

I think you should drop a mail to internal discuss this... as it's a new feature , thanks

@tony2001

This comment has been minimized.

Copy link
Contributor

commented Oct 14, 2015

@laruence I don't think it something that should be discussed in internals@. Yes, it's a new thing, but it's a really tiny feature that can't break anything.

@smalyshev

This comment has been minimized.

Copy link
Contributor

commented Oct 17, 2015

This should be at least mentioned in UPGRADING - and also of course in the manual.

@laruence

This comment has been minimized.

Copy link
Member

commented Oct 20, 2015

@weltling I think this should be target at 7.1, what do you think? only merge to master?

@KalleZ

This comment has been minimized.

Copy link
Member

commented Oct 20, 2015

@laruence I second that, 7.0.0 is way too long in the release cycle for this, could maybe come in a patch release at one point as its rather small, but for now I think it should target 7.1 aswell

@laruence

This comment has been minimized.

Copy link
Member

commented Oct 20, 2015

okey, maybe 7.0.1 is also okey... thanks

@weltling

This comment has been minimized.

Copy link
Contributor

commented Oct 20, 2015

@laruence, @kaja47 yeah, lets take it into 7.0.1, nowadays that people can use pthreads directly from PHP :)

@tony2001

This comment has been minimized.

Copy link
Contributor

commented Feb 2, 2016

@weltling any news on this one?

@weltling

This comment has been minimized.

Copy link
Contributor

commented Feb 2, 2016

As mentioned, I would see it fine with early 7.0, small feature that unlikely to break things but can be useful in several cases.

This issue unfortunately gone down from my horizon in the holiday times and under the huge amount of other patches, and no one pinged to the 7.0.1 times :( I think that it is still an early enough stage to include this feature, but i'd unlikely to have time to track properly. @tony2001 you're interested in this, please feel free to target 7.0.4. If you commit this, could you please care that it's properly documented in NEWS, UGRADING and the manual? The documentation is badly needed, if this is getting in.

Thanks.

@krakjoe

This comment has been minimized.

Copy link
Member

commented Jan 1, 2017

@weltling still okay for 7.0 ?

@eelf please could you prepare patch as suggested (news/upgrading and appropriate target) :)

@weltling

This comment has been minimized.

Copy link
Contributor

commented Jan 2, 2017

@krakjoe yeah, I think it should be still ok for 7.0, technically. Seems no further work is planned by the author, which is a pity. If no one comes to this earlier, i can take some day for merge and docs. Otherwise, could be done anytime with master, probably, which would guarantee a much better visibility of this new feature. Anyway, the piece is useful and should be picked up, not forgotten like last time.

Thanks.

@krakjoe

This comment has been minimized.

Copy link
Member

commented Jan 4, 2017

@weltling agree, let's give the author another couple of days from now to work on it, if not, I'll merge into master and we can both take it from there ;)

@weltling

This comment has been minimized.

Copy link
Contributor

commented Jan 7, 2017

Merged with d027924, doc to follow.

Thanks.

@weltling weltling closed this Jan 7, 2017

salathe pushed a commit to salathe/phpdoc-en that referenced this pull request Jan 7, 2017

extend doc for php/php-src#1546
git-svn-id: https://svn.php.net/repository/phpdoc/en/trunk@341610 c90b9560-bf6c-de11-be94-00142212c4b1

LawnGnome pushed a commit to LawnGnome/phpdoc-en that referenced this pull request Jan 7, 2017

svn2github pushed a commit to svn2github/phpdoc_en that referenced this pull request Jan 7, 2017

ab
extend doc for php/php-src#1546
git-svn-id: http://svn.php.net/repository/phpdoc/en@341610 c90b9560-bf6c-de11-be94-00142212c4b1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.