Revise buffer/seek code a little.

Tidy up user streams even more.
Make test case quite aggressive.

ext/standard/tests/file/userstreams.phpt

@@ -4,28 +4,151 @@ User-space streams
 <?php
 # vim600:syn=php:
 
-class uselessstream {
+/* This is a fairly aggressive test that looks at
+ * user streams and also gives the seek/gets/buffer
+ * layer of streams a thorough testing */
+
+$lyrics = <<<EOD
+...and the road becomes my bride
+I have stripped of all but pride
+so in her I do confide
+and she keeps me satisfied
+gives me all I need
+...and with dust in throat I crave
+to the game you stay a slave
+rover  wanderer
+nomad  vagabond
+call me what you will
+   But Ill take my time anywhere
+   Free to speak my mind anywhere
+   and Ill redefine anywhere
+      Anywhere I roam
+	  Where I lay my head is home
+...and the earth becomes my throne
+I adapt to the unknown
+under wandering stars Ive grown
+by myself but not alone
+I ask no one
+...and my ties are severed clean
+the less I have the more I gain
+off the beaten path I reign
+rover  wanderer
+nomad  vagabond
+call me what you will
+   But Ill take my time anywhere
+   Free to speak my mind anywhere
+   and Ill never mind anywhere
+      Anywhere I roam
+	  Where I lay my head is home
+   But Ill take my time anywhere
+   Free to speak my mind anywhere
+   and Ill take my find anywhere
+      Anywhere I roam
+	  Where I lay my head is home
+   carved upon my stone
+   my body lie but still I roam
+      Wherever I may roam.
+
+Wherever I May Roam
+
+EOD;
+
+/* repeat the data a few times so that it grows larger than
+ * the default cache chunk size and that we have something
+ * to seek around... */
+$DATA = "";
+for ($i = 0; $i < 30; $i++) {
+	if ($i % 2 == 0)
+		$DATA .= str_rot13($lyrics);
+	else
+		$DATA .= $lyrics;
 }
 
-class mystream {
+/* store the data in a regular file so that we can compare
+ * the results */
+$tf = tmpfile();
+fwrite($tf, $DATA);
+$n = ftell($tf);
+rewind($tf) or die("failed to rewind tmp file!");
+if (ftell($tf) != 0)
+	die("tmpfile is not at start!");
+$DATALEN = strlen($DATA);
+if ($n != $DATALEN)
+	die("tmpfile stored $n bytes; should be $DATALEN!");
 
-	function mystream()
-	{
-		echo "MYSTREAM: constructor called!\n";
-	}
+class uselessstream {
+}
 
+class mystream {
 	var $path;
 	var $mode;
 	var $options;
 
+	var $position;
+	var $varname;
+
 	function stream_open($path, $mode, $options, &$opened_path)
 	{
 		$this->path = $path;
 		$this->mode = $mode;
 		$this->options = $options;
+
+		$split = parse_url($path);
+		$this->varname = $split["host"];
+		$this->position = 0;
+		
 		return true;
 	}
 
+	function stream_read($count)
+	{
+		$ret = substr($GLOBALS[$this->varname], $this->position, $count);
+		$this->position += strlen($ret);
+		return $ret;
+	}
+
+	function stream_tell()
+	{
+		return $this->position;
+	}
+
+	function stream_eof()
+	{
+		return $this->position >= strlen($GLOBALS[$this->varname]);
+	}
+
+	function stream_seek($offset, $whence)
+	{
+		switch($whence) {
+			case SEEK_SET:
+				if ($offset < strlen($GLOBALS[$this->varname]) && $offset >= 0) {
+					$this->position = $offset;
+					return true;
+				} else {
+					return false;
+				}
+				break;
+			case SEEK_CUR:
+				if ($offset >= 0) {
+					$this->position += $offset;
+					return true;
+				} else {
+					return false;
+				}
+				break;
+			case SEEK_END:
+				if (strlen($GLOBALS[$this->varname]) + $offset >= 0) {
+					$this->position = strlen($GLOBALS[$this->varname]) + $offset;
+					return true;
+				} else {
+					return false;
+				}
+				break;
+			default:
+				return false;
+		}
+	}
+
 }
 
 if (@file_register_wrapper("bogus", "class_not_exist"))
@@ -42,10 +165,129 @@ $b = @fopen("bogon://url", "rb");
 if (is_resource($b))
 	die("Opened a bogon??");
 
-$fp = fopen("test://url", "rb");
+$fp = fopen("test://DATA", "rb");
 if (!is_resource($fp))
 	die("Failed to open resource");
 
+/* some default seeks that will cause buffer/cache misses */
+$seeks = array(
+	array(SEEK_SET, 0, 0),
+	array(SEEK_CUR, 8450, 8450),
+	array(SEEK_CUR, -7904, 546),
+	array(SEEK_CUR, 12456, 13002),
+
+	/* end up at BOF so that randomly generated seek offsets
+	 * below will know where they are supposed to be */
+	array(SEEK_SET, 0, 0)
+);
+
+$whence_map = array(
+	SEEK_CUR,
+	SEEK_SET,
+	SEEK_END
+);
+$whence_names = array(
+	SEEK_CUR => "SEEK_CUR",
+	SEEK_SET => "SEEK_SET",
+	SEEK_END => "SEEK_END"
+	);
+
+/* generate some random seek offsets */
+$position = 0;
+for ($i = 0; $i < 256; $i++) {
+	$whence = $whence_map[array_rand($whence_map, 1)];
+	switch($whence) {
+		case SEEK_SET:
+			$offset = rand(0, $DATALEN);
+			$position = $offset;
+			break;
+		case SEEK_END:
+			$offset = -rand(0, $DATALEN);
+			$position = $DATALEN + $offset;
+			break;
+		case SEEK_CUR:
+			$offset = rand(0, $DATALEN);
+			$offset -= $position;
+			$position += $offset;
+			break;
+	}
+
+	$seeks[] = array($whence, $offset, $position);
+}
+
+/* we compare the results of fgets using differing line lengths to 
+ * test the fgets layer also */
+$line_lengths = array(1024, 256, 64, 16);
+$fail_count = 0;
+
+ob_start();
+foreach($line_lengths as $line_length) {
+	/* now compare the real stream with the user stream */
+	$j = 0;
+	rewind($tf);
+	rewind($fp);
+	foreach($seeks as $seekdata) {
+		list($whence, $offset, $position) = $seekdata;
+
+		$rpb = ftell($tf);
+		$rr = (int)fseek($tf, $offset, $whence);
+		$rpa = ftell($tf);
+		$rline = fgets($tf, $line_length);
+		(int)fseek($tf, - strlen($rline), SEEK_CUR);
+
+		$upb = ftell($fp);
+		$ur = (int)fseek($fp, $offset, $whence);
+		$upa = ftell($fp);
+		$uline = fgets($fp, $line_length);
+		(int)fseek($fp, - strlen($uline), SEEK_CUR);
+
+		printf("\n--[%d] whence=%s offset=%d line_length=%d position_should_be=%d --\n",
+			$j, $whence_names[$whence], $offset, $line_length, $position);
+		printf("REAL: pos=(%d,%d,%d) ret=%d line=`%s'\n", $rpb, $rpa, ftell($tf), $rr, $rline);
+		printf("USER: pos=(%d,%d,%d) ret=%d line=`%s'\n", $upb, $upa, ftell($fp), $ur, $uline);
+
+		if ($rr != $ur || $rline != $uline || $rpa != $position || $upa != $position) {
+			$fail_count++;
+			$dat = file_get_wrapper_data($fp);
+			var_dump($dat);
+			break;
+		}
+		
+		$j++;
+	}
+	if ($fail_count)
+		break;
+}
+
+if ($fail_count == 0) {
+	ob_end_clean();
+	echo "SEEK: OK\n";
+} else {
+	echo "SEEK: FAIL\n";
+	ob_end_flush();
+}
+
+$fail_count = 0;
+fseek($fp, $DATALEN / 2, SEEK_SET);
+fseek($tf, $DATALEN / 2, SEEK_SET);
+
+while(!feof($fp)) {
+	$uline = fgets($fp, 1024);
+	$rline = fgets($fp, 1024);
+
+	if ($uline != $rline) {
+		echo "FGETS: FAIL\nuser=$uline\nreal=$rline\n";
+		$fail_count++;
+		break;
+	}
+}
+
+if ($fail_count == 0)
+	echo "FGETS: OK\n";
+
+
 ?>
 --EXPECT--
 Registered
+SEEK: OK
+FGETS: OK

main/memory_streams.c

@@ -216,6 +216,7 @@ php_stream_ops	php_stream_memory_ops = {
 PHPAPI php_stream *_php_stream_memory_create(int mode STREAMS_DC TSRMLS_DC)
 {
 	php_stream_memory_data *self;
+	php_stream *stream;
 
 	self = emalloc(sizeof(*self));
 	assert(self != NULL);
@@ -224,7 +225,10 @@ PHPAPI php_stream *_php_stream_memory_create(int mode STREAMS_DC TSRMLS_DC)
 	self->fsize = 0;
 	self->smax = -1;
 	self->mode = mode;
-	return php_stream_alloc(&php_stream_memory_ops, self, 0, "rwb");
+	
+	stream = php_stream_alloc(&php_stream_memory_ops, self, 0, "rwb");
+	stream->flags |= PHP_STREAM_FLAG_NO_BUFFER;
+	return stream;
 }
 /* }}} */
 
@@ -435,8 +439,9 @@ PHPAPI php_stream *_php_stream_temp_create(int mode, size_t max_memory_usage STR
 	self->smax = max_memory_usage;
 	self->mode = mode;
 	stream = php_stream_alloc(&php_stream_temp_ops, self, 0, "rwb");
+	stream->flags |= PHP_STREAM_FLAG_NO_BUFFER;
 	self->innerstream = php_stream_memory_create(mode);
-/*	php_stream_temp_write(stream, NULL, 0 TSRMLS_CC); */
+
 	return stream;
 }
 /* }}} */

main/php_streams.h

@@ -377,6 +377,10 @@ PHPAPI int _php_stream_set_option(php_stream *stream, int option, int value, voi
 #define PHP_STREAM_BUFFER_LINE	1	/* line buffered */
 #define PHP_STREAM_BUFFER_FULL	2	/* fully buffered */
 
+#define PHP_STREAM_OPTION_RETURN_OK			 0 /* option set OK */
+#define PHP_STREAM_OPTION_RETURN_ERR 		-1 /* problem setting option */
+#define PHP_STREAM_OPTION_RETURN_NOTIMPL	-2 /* underlying stream does not implement; streams can handle it instead */
+
 /* copy up to maxlen bytes from src to dest.  If maxlen is PHP_STREAM_COPY_ALL, copy until eof(src).
  * Uses mmap if the src is a plain file and at offset 0 */
 #define PHP_STREAM_COPY_ALL		-1

main/streams.c

@@ -357,8 +357,36 @@ static void php_stream_fill_read_buffer(php_stream *stream, size_t size TSRMLS_D
 
 PHPAPI size_t _php_stream_read(php_stream *stream, char *buf, size_t size TSRMLS_DC)
 {
+	size_t avail, toread, didread = 0;
+	
+	/* take from the read buffer first.
+	 * It is possible that a buffered stream was switched to non-buffered, so we
+	 * drain the remainder of the buffer before using the "raw" read mode for
+	 * the excess */
+	avail = stream->writepos - stream->readpos;
+	if (avail) {
+		toread = avail;
+		if (toread > size)
+			toread = size;
+
+		memcpy(buf, stream->readbuf + stream->readpos, toread);
+		stream->readpos += toread;
+		size -= toread;
+		buf += toread;
+		didread += size;
+	}
+
+	if (size == 0)
+		return didread;
+	
 	if (stream->flags & PHP_STREAM_FLAG_NO_BUFFER || stream->chunk_size == 1) {
-		return stream->ops->read(stream, buf, size TSRMLS_CC);
+		if (stream->filterhead) {
+			didread += stream->filterhead->fops->read(stream, stream->filterhead,
+					buf, size
+					TSRMLS_CC);
+		} else {
+			didread += stream->ops->read(stream, buf, size TSRMLS_CC);
+		}
 	} else {
 		php_stream_fill_read_buffer(stream, size TSRMLS_CC);
 
@@ -367,9 +395,10 @@ PHPAPI size_t _php_stream_read(php_stream *stream, char *buf, size_t size TSRMLS
 
 		memcpy(buf, stream->readbuf + stream->readpos, size);
 		stream->readpos += size;
-		stream->position += size;
-		return size;
+		didread += size;
 	}
+	stream->position += size;
+	return didread;
 }
 
 PHPAPI int _php_stream_eof(php_stream *stream TSRMLS_DC)
@@ -550,7 +579,7 @@ PHPAPI off_t _php_stream_tell(php_stream *stream TSRMLS_DC)
 PHPAPI int _php_stream_seek(php_stream *stream, off_t offset, int whence TSRMLS_DC)
 {
 	/* not moving anywhere */
-	if (offset == 0 && whence == SEEK_CUR)
+	if ((offset == 0 && whence == SEEK_CUR) || (offset == stream->position && whence == SEEK_SET))
 		return 0;
 
 	/* handle the case where we are in the buffer */
@@ -576,6 +605,8 @@ PHPAPI int _php_stream_seek(php_stream *stream, off_t offset, int whence TSRMLS_
 	stream->readpos = stream->writepos = 0;
 
 	if (stream->ops->seek && (stream->flags & PHP_STREAM_FLAG_NO_SEEK) == 0) {
+		int ret;
+		
 		if (stream->filterhead)
 			stream->filterhead->fops->flush(stream, stream->filterhead, 0 TSRMLS_CC);
 		
@@ -585,7 +616,12 @@ PHPAPI int _php_stream_seek(php_stream *stream, off_t offset, int whence TSRMLS_
 				whence = SEEK_SET;
 				break;
 		}
-		return stream->ops->seek(stream, offset, whence, &stream->position TSRMLS_CC);
+		ret = stream->ops->seek(stream, offset, whence, &stream->position TSRMLS_CC);
+
+		if (((stream->flags & PHP_STREAM_FLAG_NO_SEEK) == 0) || ret == 0)
+			return ret;
+		/* else the stream has decided that it can't support seeking after all;
+		 * fall through to attempt emulation */
 	}
 
 	/* emulate forward moving seeks with reads */
@@ -603,16 +639,37 @@ PHPAPI int _php_stream_seek(php_stream *stream, off_t offset, int whence TSRMLS_
 		return 0;
 	}
 
-	php_error_docref(NULL TSRMLS_CC, E_WARNING, "streams of type %s do not support seeking", stream->ops->label);
+	php_error_docref(NULL TSRMLS_CC, E_WARNING, "stream does not support seeking");
 
 	return -1;
 }
 
 PHPAPI int _php_stream_set_option(php_stream *stream, int option, int value, void *ptrparam TSRMLS_DC)
 {
-	if (stream->ops->set_option)
-		return stream->ops->set_option(stream, option, value, ptrparam TSRMLS_CC);
-	return -1;
+	int ret = PHP_STREAM_OPTION_RETURN_NOTIMPL;
+	
+	if (stream->ops->set_option) {
+		ret = stream->ops->set_option(stream, option, value, ptrparam TSRMLS_CC);
+	}
+	
+	if (ret == PHP_STREAM_OPTION_RETURN_NOTIMPL) {
+		switch(option) {
+			case PHP_STREAM_OPTION_BUFFER:
+				/* try to match the buffer mode as best we can */
+				if (value == PHP_STREAM_BUFFER_NONE) {
+					stream->flags |= PHP_STREAM_FLAG_NO_BUFFER;
+				} else {
+					stream->flags ^= PHP_STREAM_FLAG_NO_BUFFER;
+				}
+				ret = PHP_STREAM_OPTION_RETURN_OK;
+				break;
+				
+			default:
+				ret = PHP_STREAM_OPTION_RETURN_ERR;
+		}
+	}
+	
+	return ret;
 }
 
 PHPAPI size_t _php_stream_passthru(php_stream * stream STREAMS_DC TSRMLS_DC)
@@ -1097,12 +1154,15 @@ static int php_stdiop_set_option(php_stream *stream, int option, int value, void
 
 			switch(value) {
 				case PHP_STREAM_BUFFER_NONE:
+					stream->flags |= PHP_STREAM_FLAG_NO_BUFFER;
 					return setvbuf(data->file, NULL, _IONBF, 0);
 					
 				case PHP_STREAM_BUFFER_LINE:
+					stream->flags ^= PHP_STREAM_FLAG_NO_BUFFER;
 					return setvbuf(data->file, NULL, _IOLBF, size);
 					
 				case PHP_STREAM_BUFFER_FULL:
+					stream->flags ^= PHP_STREAM_FLAG_NO_BUFFER;
 					return setvbuf(data->file, NULL, _IOFBF, size);
 
 				default:

main/user_streams.c

@@ -85,29 +85,46 @@ typedef struct _php_userstream_data php_userstream_data_t;
 
 /* class should have methods like these:
  
-function stream_open($path, $mode, $options, &$opened_path)
-   {
-      return true/false;
-   }
-   function stream_read($count)
-   {
-      return false on error;
-      else return string;
-   }
-   function stream_write($data)
-   {
-      return false on error;
-      else return count written;
-   }
-   function stream_close()
-   {
-   }
-   function stream_flush()
-   {
-   }
-   function stream_seek($offset, $whence)
-   {
-   }
+	function stream_open($path, $mode, $options, &$opened_path)
+	{
+	  	return true/false;
+	}
+	
+	function stream_read($count)
+	{
+	   	return false on error;
+		else return string;
+	}
+	
+	function stream_write($data)
+	{
+	   	return false on error;
+		else return count written;
+	}
+	
+	function stream_close()
+	{
+	}
+	
+	function stream_flush()
+	{
+		return true/false;
+	}
+	
+	function stream_seek($offset, $whence)
+	{
+		return true/false;
+	}
+
+	function stream_tell()
+	{
+		return (int)$position;
+	}
+
+	function stream_eof()
+	{
+		return true/false;
+	}
   
  **/
 
@@ -267,6 +284,9 @@ static size_t php_userstreamop_write(php_stream *stream, const char *buf, size_t
 	if (call_result == SUCCESS && retval != NULL) {
 		convert_to_long_ex(&retval);
 		didwrite = Z_LVAL_P(retval);
+	} else if (call_result == FAILURE) {
+		php_error_docref(NULL TSRMLS_CC, E_WARNING, "%s::" USERSTREAM_WRITE " - is not implemented!",
+				us->wrapper->classname);
 	}
 
 	/* don't allow strange buffer overruns due to bogus return */
@@ -305,8 +325,14 @@ static size_t php_userstreamop_read(php_stream *stream, char *buf, size_t count
 
 		if (call_result == SUCCESS && retval != NULL && zval_is_true(retval))
 			didread = 0;
-		else
+		else {
+			if (call_result == FAILURE) {
+				php_error_docref(NULL TSRMLS_CC, E_WARNING, "%s::" USERSTREAM_EOF " - is not implemented! Assuming EOF",
+						us->wrapper->classname);
+			}
+
 			didread = EOF;
+		}
 
 	} else {
 		zval *zcount;
@@ -334,8 +360,10 @@ static size_t php_userstreamop_read(php_stream *stream, char *buf, size_t count
 			}
 			if (didread > 0)
 				memcpy(buf, Z_STRVAL_P(retval), didread);
+		} else if (call_result == FAILURE) {
+			php_error_docref(NULL TSRMLS_CC, E_WARNING, "%s::" USERSTREAM_READ " - is not implemented!",
+					us->wrapper->classname);
 		}
-
 		zval_ptr_dtor(&zcount);
 	}
 	
@@ -430,10 +458,19 @@ static int php_userstreamop_seek(php_stream *stream, off_t offset, int whence, o
 	zval_ptr_dtor(&zoffs);
 	zval_ptr_dtor(&zwhence);
 
-	if (call_result == SUCCESS && retval != NULL && Z_TYPE_P(retval) == IS_LONG)
-		ret = Z_LVAL_P(retval);
-	else
+	if (call_result == FAILURE) {
+		/* stream_seek is not implemented, so disable seeks for this stream */
+		stream->flags |= PHP_STREAM_FLAG_NO_SEEK;
+		/* there should be no retval to clean up */
+		return -1;
+	} else if (call_result == SUCCESS && retval != NULL && zval_is_true(retval)) {
+		ret = 0;
+	} else {
 		ret = -1;
+	}
+
+	if (retval)
+		zval_ptr_dtor(&retval);
 
 	/* now determine where we are */
 	ZVAL_STRINGL(&func_name, USERSTREAM_TELL, sizeof(USERSTREAM_TELL)-1, 0);
@@ -446,6 +483,9 @@ static int php_userstreamop_seek(php_stream *stream, off_t offset, int whence, o
 
 	if (call_result == SUCCESS && retval != NULL && Z_TYPE_P(retval) == IS_LONG)
 		*newoffs = Z_LVAL_P(retval);
+	else
+		php_error_docref(NULL TSRMLS_CC, E_WARNING, "%s::" USERSTREAM_TELL " - is not implemented!",
+				us->wrapper->classname);
 	
 	if (retval)
 		zval_ptr_dtor(&retval);