Improve the general behaviour of stream_gets and fix its semantics

with regard to sockets. The behaviour should be aligned with PHP 4.2 now.
This has been verified to some degree.

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.

diff --git a/main/network.c b/main/network.c
index 484550ac5df05833a67e34ac4816bac2c9a18e3e..fad7e40ff5a0b6249ccfa938c6117d0f6ed1b06e 100644 (file)
--- a/main/network.c
+++ b/main/network.c
@@ -923,7 +923,8 @@ php_stream_ops php_stream_socket_ops = {
        NULL, /* seek */
        php_sockop_cast,
        php_sockop_stat,
-       php_sockop_set_option
+       php_sockop_set_option,
+       1
 };
 
 

diff --git a/main/php_streams.h b/main/php_streams.h
index 3ae9455bc24b9e2d60f70b41c29ed75c2ccacfad..421dbf253583db21408ccceff138650009d80331 100755 (executable)
--- a/main/php_streams.h
+++ b/main/php_streams.h
@@ -153,6 +153,7 @@ typedef struct _php_stream_ops  {
        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 {

diff --git a/main/streams.c b/main/streams.c
index 5d2188f746d234e289e3963200a63ad75d58a5cf..2da448aa6488e4ccce952ef6e194914b5d1a7714 100755 (executable)
--- a/main/streams.c
+++ b/main/streams.c
@@ -494,7 +494,11 @@ static void php_stream_fill_read_buffer(php_stream *stream, size_t size TSRMLS_D
                
                if (justread <= 0)
                        break;
+               
                stream->writepos += justread;
+               
+               if (stream->ops->dont_block)
+                       break;
        }
 }
 
@@ -615,74 +619,105 @@ PHPAPI int _php_stream_stat(php_stream *stream, php_stream_statbuf *ssb TSRMLS_D
        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;
+                       int done = 0;
+
+                       readptr = stream->readbuf + stream->readpos;
+                       eol = php_stream_locate_eol(stream TSRMLS_CC);
+
+                       if (eol) {
+                               cpysz = eol - readptr + 1;
+                               done = 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;
-               }
+                       if (cpysz >= maxlen - 1) {
+                               cpysz = maxlen - 1;
+                               done = 1;
+                       }
 
-               memcpy(buf, readptr, justread);
-               didread += justread;
-               buf += justread;
-               stream->readpos += justread;
+                       memcpy(buf, readptr, cpysz);
 
-               if (eol)
-                       break;
-       }
+                       stream->position += cpysz;
+                       stream->readpos += cpysz;
+                       buf += cpysz;
+                       maxlen -= cpysz;
 
-       if (didread == 0)
-               return NULL;
+                       if (done) {
+                               break;
+                       }
+               } else {
+                       size_t toread = maxlen - 1;
+                       if (toread > stream->chunk_size)
+                               toread = stream->chunk_size;
+
+                       /* 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;
 }