Here is a second patch, with a third coming some time later.
The current stream implementation does not consume its
buffered data before starting new reads. That causes serious
problems with sockets, of course, if you have an application
which exchanges commands/replies with a server. The
stream_gets function reads many lines, returns a single one,
and then tries to read from the network again.
The patch changes the semantic: Consume the buffer first,
then call the stream read op.
My fgets-based pop3 reader works with this patch already. I
suspect that fread exposes similar problems though, but I
don't have time right now to improve it similarly.
- Sascha
Index: network.c
===================================================================
RCS file: /repository/php4/main/network.c,v
retrieving revision 1.70
diff -u -r1.70 network.c
--- network.c 28 Sep 2002 22:12:23 -0000 1.70
+++ network.c 4 Oct 2002 14:38:15 -0000
@@ -923,7 +923,8 @@
NULL, /* seek */
php_sockop_cast,
php_sockop_stat,
- php_sockop_set_option
+ php_sockop_set_option,
+ 1
};
Index: php_streams.h
===================================================================
RCS file: /repository/php4/main/php_streams.h,v
retrieving revision 1.51
diff -u -r1.51 php_streams.h
--- php_streams.h 28 Sep 2002 22:10:47 -0000 1.51
+++ php_streams.h 4 Oct 2002 14:38:16 -0000
@@ -153,6 +153,7 @@
int (*cast)(php_stream *stream, int castas, void **ret TSRMLS_DC);
int (*stat)(php_stream *stream, php_stream_statbuf *ssb TSRMLS_DC);
int (*set_option)(php_stream *stream, int option, int value, void *ptrparam
TSRMLS_DC);
+ int dont_block;
} php_stream_ops;
typedef struct _php_stream_wrapper_ops {
Index: streams.c
===================================================================
RCS file: /repository/php4/main/streams.c,v
retrieving revision 1.89
diff -u -r1.89 streams.c
--- streams.c 3 Oct 2002 16:06:41 -0000 1.89
+++ streams.c 4 Oct 2002 14:38:17 -0000
@@ -494,7 +494,11 @@
if (justread <= 0)
break;
+
stream->writepos += justread;
+
+ if (stream->ops->dont_block)
+ break;
}
}
@@ -615,74 +619,103 @@
return stream->ops->stat(stream, ssb TSRMLS_CC);
}
-PHPAPI char *_php_stream_gets(php_stream *stream, char *buf, size_t maxlen TSRMLS_DC)
+static char *php_stream_locate_eol(php_stream *stream TSRMLS_DC)
{
+ size_t avail;
char *cr, *lf, *eol = NULL;
- size_t toread = 0, didread = 0, justread = 0, avail = 0;
char *readptr;
+ readptr = stream->readbuf + stream->readpos;
+ avail = stream->writepos - stream->readpos;
+
+ /* Look for EOL */
+ if (stream->flags & PHP_STREAM_FLAG_DETECT_EOL) {
+ cr = memchr(readptr, '\r', avail);
+ lf = memchr(readptr, '\n', avail);
+
+ if (cr && lf != cr + 1) {
+ /* mac */
+ stream->flags ^= PHP_STREAM_FLAG_DETECT_EOL;
+ stream->flags |= PHP_STREAM_FLAG_EOL_MAC;
+ eol = cr;
+ } else if ((cr && lf && cr == lf - 1) || (lf)) {
+ /* dos or unix endings */
+ stream->flags ^= PHP_STREAM_FLAG_DETECT_EOL;
+ eol = lf;
+ }
+ } else if (stream->flags & PHP_STREAM_FLAG_EOL_MAC) {
+ eol = memchr(readptr, '\r', avail);
+ } else {
+ /* unix (and dos) line endings */
+ eol = memchr(readptr, '\n', avail);
+ }
+
+ return eol;
+}
+
+PHPAPI char *_php_stream_gets(php_stream *stream, char *buf, size_t maxlen TSRMLS_DC)
+{
+ size_t avail = 0;
+
if (maxlen == 0)
return NULL;
-
- while (didread < maxlen - 1) {
- toread = maxlen - 1;
- if (toread > stream->chunk_size)
- toread = stream->chunk_size;
- php_stream_fill_read_buffer(stream, toread TSRMLS_CC);
+ /*
+ * If the underlying stream operations block when no new data is readable,
+ * we need to take extra precautions.
+ *
+ * If there is buffered data available, we check for a EOL. If it exists,
+ * we pass the data immediately back to the caller. This saves a call
+ * to the read implementation and will not block where blocking
+ * is not necessary at all.
+ *
+ * If the stream buffer contains more data than the caller requested,
+ * we can also avoid that costly step and simply return that data.
+ */
- readptr = stream->readbuf + stream->readpos;
+ for (;;) {
avail = stream->writepos - stream->readpos;
- if (avail == 0)
- break;
-
- /* Look for EOL */
- if (stream->flags & PHP_STREAM_FLAG_DETECT_EOL) {
- cr = memchr(readptr, '\r', avail);
- lf = memchr(readptr, '\n', avail);
-
- if (cr && lf != cr + 1) {
- /* mac */
- stream->flags ^= PHP_STREAM_FLAG_DETECT_EOL;
- stream->flags |= PHP_STREAM_FLAG_EOL_MAC;
- eol = cr;
- } else if ((cr && lf && cr == lf - 1) || (lf)) {
- /* dos or unix endings */
- stream->flags ^= PHP_STREAM_FLAG_DETECT_EOL;
- eol = lf;
+ if (avail > 0) {
+ size_t cpysz = 0;
+ char *readptr;
+ char *eol;
+
+ readptr = stream->readbuf + stream->readpos;
+ eol = php_stream_locate_eol(stream TSRMLS_CC);
+
+ if (eol) {
+ cpysz = eol - readptr + 1;
+
+ if (cpysz >= maxlen - 1)
+ cpysz = maxlen - 1;
+ } else if (avail > maxlen - 1) {
+ cpysz = maxlen - 1;
+ } else {
+ cpysz = avail;
}
- } else if (stream->flags & PHP_STREAM_FLAG_EOL_MAC) {
- eol = memchr(readptr, '\r', avail);
- } else {
- /* unix (and dos) line endings */
- eol = memchr(readptr, '\n', avail);
- }
- if (eol && (size_t)((ptrdiff_t)eol + 1 - (ptrdiff_t)readptr) <= maxlen
- 1) {
- justread = eol + 1 - readptr;
- } else {
- eol = NULL;
- justread = toread;
- if (justread > avail)
- justread = avail;
- }
+ memcpy(buf, readptr, cpysz);
- memcpy(buf, readptr, justread);
- didread += justread;
- buf += justread;
- stream->readpos += justread;
+ stream->position += cpysz;
+ stream->readpos += cpysz;
+ buf += cpysz;
+ maxlen -= cpysz;
- if (eol)
- break;
- }
+ if (eol) {
+ break;
+ }
+ } else {
+ size_t toread = maxlen - 1;
+ if (toread > stream->chunk_size)
+ toread = stream->chunk_size;
- if (didread == 0)
- return NULL;
+ /* XXX: Should not the loop end, if the stream op fails? */
+ php_stream_fill_read_buffer(stream, toread TSRMLS_CC);
+ }
+ }
- /* terminate the buffer */
- *buf = '\0';
- stream->position += didread;
+ buf[0] = '\0';
return buf;
}
--
PHP Development Mailing List <http://www.php.net/>
To unsubscribe, visit: http://www.php.net/unsub.php
