On Sat, 07 May 2011 00:48:46 +0100, Ferenc Kovacs <i...@tyrael.hu> wrote:
On Sat, May 7, 2011 at 1:40 AM, Ferenc Kovacs <i...@tyrael.hu> wrote:
On Thu, May 5, 2011 at 2:30 AM, Ferenc Kovacs <i...@tyrael.hu> wrote:
On Sat, Apr 30, 2011 at 5:23 PM, Ferenc Kovacs <i...@tyrael.hu> wrote:
I actually did consider adding support for an extended form of
php://fd where one would specify the desired file descriptor:
php://fd/<orig fd>/<new fd>. It would call dup2 instead of dup.
However, I got into some trouble on shutdown because this could cause
stdout to be closed ahead of time and then the output subsystem
would cause either a segfault or a memory leak (can't recall). I
didn't spend more than 20 minutes on this as it was not the problem
I was trying to solve, so there's probably an easy solution. If you
want to work on this, extending php://fd would likely be a good
place.
adding dup2 support for the php://fd stream would be better than the
current situation, but I can't see why are we trying to force
everything into that.
I mean everything on the http://www.php.net/manual/en/wrappers.php
can be
achived through a function also.
there are some case when you can save a couple of lines of code with
the wrappers and with http://www.php.net/manual/en/context.http.php
but there is no method for opening FDs or the dup2 that you mentioned.
I think that it would be more consistent with the rest of the
language, and one would more likely to find fdopen than
fopen('php://fd/1'); in the documentation.
I just don't see the point of duplicating functionality, especially when
there's already a closely related wrapper.
It's not true that everything in http://php.net/wrappers can can be done
in another fashion. In fact, for the php:// wrappers, which is what we're
talking about, ONLY ONE (php://filter) duplicates functionality that can
be achieved with functions.
closing the stdout without reopening also caused problems for me in
case of an error happens, but this shouldn't happen if you properly
reopen it, so I would be interested what exactly happened there.
Yes, this causes a memory leak. Maybe that was the problem I found. Duping
the file descriptor after the call to dup2 would solve this, which I think
is one more reason not to expose these low-level functions directly and
provide a safer alternative (more below).
I put together the fileno, fdopen, dup2, fclose stuff as a pecl
extension(called fildes), the sourcecode available here:
https://github.com/Tyrael/php-fildes
[...]
Could somebody review the source? as I mentioned in my previous email
I had some problem here:
https://github.com/Tyrael/php-fildes/blob/master/fildes.c#L211
if I define the second argument as int, the first argument will be
always 0.
I have no idea whats going there.
See http://lxr.php.net/xref/PHP_TRUNK/README.PARAMETER_PARSING_API#72
and I forgot to ask the most important thing:
what do you think about the feature itself?
I would really like to have it in the core, but I think it would be
better to have this as functions, than adding more stream magic.
I'm not sure where would this functions fit the best.
1, it could be added to the filesystem functions (we already have some
which are only supported on some systems)
2, it could be added to the posix functions (would be logical)
3, it could be added as stream stuff (Gustavo supports this imo)
4, it could be added to the dio pecl extension (could fit in there,
but it's not in the core, and the package doesn't seem much alive)
5, it could be added to pecl as a different extension (bad idea, imo)
PHP tries to abstract away these kind of details; I don't think direct
access to low level I/O functions is appropriate in the core. The dio pecl
extension would seem more appropriate. I don't know how alive it is, but
if I recall correctly, it got a new maintainer last year.
The POSIX extension would also seem to make sense, but it seems more
focused on another class of functions and is not available on Windows.
That said, if some useful use case is not available (or is available, but
it's awkward) in PHP, we should try to fix it. If I understand correctly,
what compels you is being able to replace stdin, stdout and stderr.
I would tend to support #3; for reference here goes the change to php://fd
that would be necessary. The problem is, as far as I know, there's no way
to retrieve the file descriptor associated a stream, so we'd still have to
rely on opening order.
One possible solution would be to add this information to the wrapper data
of the "plain files" and in systems other than Windows, to other relevant
stream types (e.g. sockets). I think your solution of calling stream_cast
is a good option because in Windows the socket streams return socket
handles -- file descriptors are an abstraction in the C lib, not really
part of the windows API -- and because in some stream implementations that
call has important side effects.
Another option would option would be to unburden the user from dealing
with file descriptors completely and provide a function to replace the
standard file descriptors with a stream.
--
Gustavo Lopes
Index: php_fopen_wrapper.c
===================================================================
--- php_fopen_wrapper.c (revision 310834)
+++ php_fopen_wrapper.c (working copy)
@@ -260,16 +260,30 @@
} else if (!strncasecmp(path, "fd/", 3)) {
char *start,
*end;
- long fildes_ori;
+ long fildes_ori,
+ fildes_des;
int dtablesize;
+ int has_des = 0;
start = &path[3];
fildes_ori = strtol(start, &end, 10);
- if (end == start || *end != '\0') {
+ if (end == start || (*end != '\0' && *end != '/')) {
php_stream_wrapper_log_error(wrapper, options TSRMLS_CC,
- "php://fd/ stream must be specified in the form
php://fd/<orig fd>");
+ "php://fd/ stream must be specified in the form
"
+ "php://fd/<orig fd>[/<target fd>]");
return NULL;
}
+ if (*end == '/') {
+ has_des = 1;
+ start = end + 1;
+ fildes_des = strtol(start, &end, 10);
+ if (end == start || *end != '\0') {
+ php_stream_wrapper_log_error(wrapper, options
TSRMLS_CC,
+ "php://fd/ stream must be specified in
the form "
+ "php://fd/<orig fd>[/<target fd>]");
+ return NULL;
+ }
+ }
#if HAVE_UNISTD_H
dtablesize = getdtablesize();
@@ -277,13 +291,25 @@
dtablesize = INT_MAX;
#endif
- if (fildes_ori < 0 || fildes_ori >= dtablesize) {
+ if (fildes_ori < 0 || fildes_ori >= dtablesize ||
+ (has_des && (fildes_des < 0 || fildes_des >=
dtablesize ))) {
php_stream_wrapper_log_error(wrapper, options TSRMLS_CC,
"The file descriptors must be non-negative
numbers smaller than %d", dtablesize);
return NULL;
}
- fd = dup(fildes_ori);
+ if (has_des) {
+ fd = dup2((int)fildes_ori, (int)fildes_des);
+ if (fd == -1) {
+ php_stream_wrapper_log_error(wrapper, options
TSRMLS_CC,
+ "Error dupping %ld to %ld: [%d]: %s",
fildes_ori, fildes_des,
+ errno, strerror(errno));
+ return NULL;
+ }
+ fildes_ori = fildes_des; /* we're now dupping it */
+ }
+
+ fd = dup((int)fildes_ori);
if (fd == -1) {
php_stream_wrapper_log_error(wrapper, options TSRMLS_CC,
"Error duping file descriptor %ld; possibly it
doesn't exist: "
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
