Damn it; my lovely resilient scheme falls down when you have a protocol

that switches from line-oriented to byte-oriented partway through, such as
HTTP.

3 files changed, 61 insertions, 85 deletions

diff --git a/src/core/linebuf.c b/src/core/linebuf.c
index 20f2c423d..ae72796c7 100644
--- a/src/core/linebuf.c
+++ b/src/core/linebuf.c
@@ -30,33 +30,13 @@
 #include <gpxe/linebuf.h>
 
 /**
- * Line terminators
+ * Retrieve buffered-up line
  *
- * These values are used in the @c skip_terminators bitmask.
- */
-enum line_terminators {
-	TERM_CR = 1,
-	TERM_NL = 2,
-	TERM_NUL = 4,
-};
-
-/**
- * Get terminator ID corresponding to character
- *
- * @v character		Character
- * @ret terminator_id	Terminator ID, or -1
+ * @v linebuf		Line buffer
+ * @ret line		Buffered line, or NULL if no line ready to read
  */
-static int terminator_id ( unsigned int character ) {
-	switch ( character ) {
-	case '\r':
-		return TERM_CR;
-	case '\n':
-		return TERM_NL;
-	case '\0':
-		return TERM_NUL;
-	default:
-		return -1;
-	}
+char * buffered_line ( struct line_buffer *linebuf ) {
+	return ( linebuf->ready ? linebuf->data : NULL );
 }
 
 /**
@@ -68,6 +48,7 @@ void empty_line_buffer ( struct line_buffer *linebuf ) {
 	free ( linebuf->data );
 	linebuf->data = NULL;
 	linebuf->len = 0;
+	linebuf->ready = 0;
 }
 
 /**
@@ -76,61 +57,60 @@ void empty_line_buffer ( struct line_buffer *linebuf ) {
  * @v linebuf			Line buffer
  * @v data			New data to add
  * @v len			Length of new data to add
- * @ret buffered		Amount of data consumed and added to the buffer
- * @ret <0			Out of memory
+ * @ret rc			Return status code
  * 
- * If line_buffer() does not consume the entirety of the new data
- * (i.e. if @c buffered is not equal to @c len), then an end of line
- * has been reached and the buffered-up line can be obtained from
- * buffered_line().  Carriage returns and newlines will have been
- * stripped, and the line will be NUL-terminated.  This buffered line
- * is valid only until the next call to line_buffer() (or to
- * empty_line_buffer()).
+ * If line_buffer() returns >0, then an end of line has been reached
+ * and the buffered-up line can be obtained from buffered_line().
+ * Carriage returns and newlines will have been stripped, and the line
+ * will be NUL-terminated.  This buffered line is valid only until the
+ * next call to line_buffer() (or to empty_line_buffer()).
+ *
+ * @c data and @c len will be updated to reflect the data consumed by
+ * line_buffer().
  *
  * Note that line buffers use dynamically allocated storage; you
  * should call empty_line_buffer() before freeing a @c struct @c
  * line_buffer.
  */
-int line_buffer ( struct line_buffer *linebuf, const char *data, size_t len ) {
-	size_t consume = 0;
-	size_t copy = 0;
+int line_buffer ( struct line_buffer *linebuf,
+		  const char **data, size_t *len ) {
+	const char *eol;
+	size_t consume;
 	size_t new_len;
 	char *new_data;
-	int terminator;
 
-	/* First, handle the termination of the previous line */
-	if ( linebuf->skip_terminators ) {
-		/* Free buffered string */
+	/* Free any completed line from previous iteration */
+	if ( linebuf->ready )
 		empty_line_buffer ( linebuf );
-		/* Skip over any terminators from the end of a previous line */
-		for ( ; consume < len ; consume++ ) {
-			terminator = terminator_id ( data[consume] );
-			if ( ( terminator < 0 ) ||
-			     ! ( linebuf->skip_terminators & terminator ) ) {
-				linebuf->skip_terminators = 0;
-				break;
-			}
-			linebuf->skip_terminators &= ~terminator;
-		}
-	}
 
-	/* Scan up to the next terminator, if any */
-	for ( ; consume < len ; consume++, copy++ ) {
-		if ( terminator_id ( data[consume] ) >= 0 ) {
-			linebuf->skip_terminators = -1U;
-			break;
-		}
+	/* Search for line terminator */
+	if ( ( eol = memchr ( *data, '\n', *len ) ) ) {
+		consume = ( eol - *data + 1 );
+	} else {
+		consume = *len;
 	}
 
 	/* Reallocate data buffer and copy in new data */
-	new_len = ( linebuf->len + copy );
+	new_len = ( linebuf->len + consume );
 	new_data = realloc ( linebuf->data, ( new_len + 1 ) );
 	if ( ! new_data )
 		return -ENOMEM;
-	memcpy ( ( new_data + linebuf->len ), ( data + consume - copy ),
-		 copy );
+	memcpy ( ( new_data + linebuf->len ), *data, consume );
 	new_data[new_len] = '\0';
 	linebuf->data = new_data;
 	linebuf->len = new_len;
-	return consume;
+
+	/* Update data and len */
+	*data += consume;
+	*len -= consume;
+
+	/* If we have reached end of line, trim the line and mark as ready */
+	if ( eol ) {
+		linebuf->data[--linebuf->len] = '\0'; /* trim NL */
+		if ( linebuf->data[linebuf->len - 1] == '\r' )
+			linebuf->data[--linebuf->len] = '\0'; /* trim CR */
+		linebuf->ready = 1;
+	}
+
+	return 0;
 }
diff --git a/src/include/gpxe/linebuf.h b/src/include/gpxe/linebuf.h
index 72b477526..f4481ddfe 100644
--- a/src/include/gpxe/linebuf.h
+++ b/src/include/gpxe/linebuf.h
@@ -12,26 +12,17 @@
 
 /** A line buffer */
 struct line_buffer {
-	/** Current data in the buffer */
+	/** Current string in the buffer */
 	char *data;
-	/** Length of current data */
+	/** Length of current string, excluding the terminating NUL */
 	size_t len;
-	/** Bitmask of terminating characters to skip over */
-	unsigned int skip_terminators;
+	/** String is ready to read */
+	int ready;
 };
 
-/**
- * Retrieve buffered-up line
- *
- * @v linebuf		Line buffer
- * @ret line		Buffered line, or NULL if no line present
- */
-static inline char * buffered_line ( struct line_buffer *linebuf ) {
-	return linebuf->data;
-}
-
-extern int line_buffer ( struct line_buffer *linebuf, const char *data,
-			 size_t len );
+extern char * buffered_line ( struct line_buffer *linebuf );
+extern int line_buffer ( struct line_buffer *linebuf,
+		  const char **data, size_t *len );
 extern void empty_line_buffer ( struct line_buffer *linebuf );
 
 #endif /* _GPXE_LINEBUF_H */
diff --git a/src/tests/linebuf_test.c b/src/tests/linebuf_test.c
index 9ddbb75a0..31ce808bf 100644
--- a/src/tests/linebuf_test.c
+++ b/src/tests/linebuf_test.c
@@ -5,8 +5,8 @@
 
 static const char data1[] = 
 "Hello world\r\n"
-"This is a particularly mean set of lines\n"
-"with a mixture of terminators\r\r\n"
+"This is a reasonably nice set of lines\n"
+"with not many different terminators\r\n\r\n"
 "There should be exactly one blank line above\n"
 "and this line should never appear at all since it has no terminator";
 
@@ -14,13 +14,18 @@ void linebuf_test ( void ) {
 	struct line_buffer linebuf;
 	const char *data = data1;
 	size_t len = ( sizeof ( data1 ) - 1 /* be mean; strip the NUL */ );
-	size_t buffered;
+	char *line;
+	int rc;
 
 	memset ( &linebuf, 0, sizeof ( linebuf ) );
-	while ( ( buffered = line_buffer ( &linebuf, data, len ) ) != len ) {
-		printf ( "\"%s\"\n", buffered_line ( &linebuf ) );
-		data += buffered;
-		len -= buffered;
+	while ( len ) {
+		if ( ( rc = line_buffer ( &linebuf, &data, &len ) ) != 0 ) {
+			printf ( "line_buffer() failed: %s\n",
+				 strerror ( rc ) );
+			return;
+		}
+		if ( ( line = buffered_line ( &linebuf ) ) )
+			printf ( "\"%s\"\n", line );
 	}
 
 	empty_line_buffer ( &linebuf );