decouple the filling of the read buffer from consuming it

this prepares the code for being called from a callback.

notably, this makes the imap list parser have a "soft stack", so the
recursion can be suspended at any time.

src/drv_imap.c

@@ -67,6 +67,13 @@ typedef struct _list {
 	int len;
 } list_t;
 
+#define MAX_LIST_DEPTH 5
+
+typedef struct parse_list_state {
+	list_t *head, **stack[MAX_LIST_DEPTH];
+	int level, need_bytes;
+} parse_list_state_t;
+
 struct imap_cmd;
 
 typedef struct imap_store {
@@ -79,6 +86,7 @@ typedef struct imap_store {
 	list_t *ns_personal, *ns_other, *ns_shared; /* NAMESPACE info */
 	message_t **msgapp; /* FETCH results */
 	unsigned caps; /* CAPABILITY results */
+	parse_list_state_t parse_list_sts;
 	/* command queue */
 	int nexttag, num_in_progress, literal_pending;
 	struct imap_cmd *in_progress, **in_progress_append;
@@ -433,66 +441,76 @@ free_list( list_t *list )
 	}
 }
 
+enum {
+	LIST_OK,
+	LIST_PARTIAL,
+	LIST_BAD
+};
+
 static int
-parse_imap_list_l( imap_store_t *ctx, char **sp, list_t **curp, int level )
+parse_imap_list( imap_store_t *ctx, char **sp, parse_list_state_t *sts )
 {
-	list_t *cur;
+	list_t *cur, **curp;
 	char *s = *sp, *p;
-	int n, bytes;
+	int bytes;
+
+	assert( sts );
+	assert( sts->level > 0 );
+	curp = sts->stack[--sts->level];
+	bytes = sts->need_bytes;
+	if (bytes >= 0) {
+		sts->need_bytes = -1;
+		if (!bytes)
+			goto getline;
+		cur = (list_t *)((char *)curp - offsetof(list_t, next));
+		s = cur->val + cur->len - bytes;
+		goto getbytes;
+	}
 
 	for (;;) {
 		while (isspace( (unsigned char)*s ))
 			s++;
-		if (level && *s == ')') {
+		if (sts->level && *s == ')') {
 			s++;
-			break;
+			curp = sts->stack[--sts->level];
+			goto next;
 		}
 		*curp = cur = nfmalloc( sizeof(*cur) );
-		curp = &cur->next;
 		cur->val = 0; /* for clean bail */
+		curp = &cur->next;
+		*curp = 0; /* ditto */
 		if (*s == '(') {
 			/* sublist */
+			if (sts->level == MAX_LIST_DEPTH)
+				goto bail;
 			s++;
 			cur->val = LIST;
-			if (parse_imap_list_l( ctx, &s, &cur->child, level + 1 ))
+			sts->stack[sts->level++] = curp;
-				goto bail;
+			curp = &cur->child;
+			*curp = 0; /* for clean bail */
+			goto next2;
 		} else if (ctx && *s == '{') {
 			/* literal */
 			bytes = cur->len = strtol( s + 1, &s, 10 );
-			if (*s != '}')
+			if (*s != '}' || *++s)
 				goto bail;
 
 			s = cur->val = nfmalloc( cur->len );
 
-			/* dump whats left over in the input buffer */
+		  getbytes:
-			n = ctx->conn.bytes - ctx->conn.offset;
+			bytes -= socket_read( &ctx->conn, s, bytes );
+			if (bytes > 0)
+				goto postpone;
 
-			if (n > bytes)
-				/* the entire message fit in the buffer */
-				n = bytes;
-
-			memcpy( s, ctx->conn.buf + ctx->conn.offset, n );
-			s += n;
-			bytes -= n;
-
-			/* mark that we used part of the buffer */
-			ctx->conn.offset += n;
-
-			/* now read the rest of the message */
-			while (bytes > 0) {
-				if ((n = socket_read( &ctx->conn, s, bytes )) <= 0)
-					goto bail;
-				s += n;
-				bytes -= n;
-			}
 			if (DFlags & XVERBOSE) {
 				puts( "=========" );
 				fwrite( cur->val, cur->len, 1, stdout );
 				puts( "=========" );
 			}
 
-			if (buffer_gets( &ctx->conn, &s ))
+		  getline:
-				goto bail;
+			if (!(s = socket_read_line( &ctx->conn )))
+				goto postpone;
 		} else if (*s == '"') {
 			/* quoted string */
 			s++;
@@ -509,7 +527,7 @@ parse_imap_list_l( imap_store_t *ctx, char **sp, list_t **curp, int level )
 			/* atom */
 			p = s;
 			for (; *s && !isspace( (unsigned char)*s ); s++)
-				if (level && *s == ')')
+				if (sts->level && *s == ')')
 					break;
 			cur->len = s - p;
 			if (cur->len == 3 && !memcmp ("NIL", p, 3))
@@ -521,41 +539,50 @@ parse_imap_list_l( imap_store_t *ctx, char **sp, list_t **curp, int level )
 			}
 		}
 
-		if (!level)
+	  next:
+		if (!sts->level)
 			break;
+	  next2:
 		if (!*s)
 			goto bail;
 	}
 	*sp = s;
-	*curp = 0;
+	return LIST_OK;
-	return 0;
 
+  postpone:
+	if (sts->level < MAX_LIST_DEPTH) {
+		sts->stack[sts->level++] = curp;
+		sts->need_bytes = bytes;
+		return LIST_PARTIAL;
+	}
   bail:
-	*curp = 0;
+	free_list( sts->head );
-	return -1;
+	return LIST_BAD;
 }
 
-static list_t *
+static void
-parse_imap_list( imap_store_t *ctx, char **sp )
+parse_list_init( parse_list_state_t *sts )
 {
-	list_t *head;
+	sts->need_bytes = -1;
-
+	sts->level = 1;
-	if (!parse_imap_list_l( ctx, sp, &head, 0 ))
+	sts->head = 0;
-		return head;
+	sts->stack[0] = &sts->head;
-	free_list( head );
-	return NULL;
 }
 
 static list_t *
 parse_list( char **sp )
 {
-	return parse_imap_list( 0, sp );
+	parse_list_state_t sts;
+	parse_list_init( &sts );
+	if (parse_imap_list( 0, sp, &sts ) == LIST_OK)
+		return sts.head;
+	return NULL;
 }
 
 static int
-parse_fetch( imap_store_t *ctx, char *cmd ) /* move this down */
+parse_fetch( imap_store_t *ctx, list_t *list )
 {
-	list_t *tmp, *list, *flags;
+	list_t *tmp, *flags;
 	char *body = 0;
 	imap_message_t *cur;
 	msg_data_t *msgdata;
@@ -563,8 +590,6 @@ parse_fetch( imap_store_t *ctx, char *cmd ) /* move this down */
 	int uid = 0, mask = 0, status = 0, size = 0;
 	unsigned i;
 
-	list = parse_imap_list( ctx, &cmd );
-
 	if (!is_list( list )) {
 		error( "IMAP error: bogus FETCH response\n" );
 		free_list( list );
@@ -784,8 +809,11 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
 
 	greeted = ctx->greeting;
 	for (;;) {
-		if (buffer_gets( &ctx->conn, &cmd ))
+		if (!(cmd = socket_read_line( &ctx->conn ))) {
+			if (socket_fill( &ctx->conn ) < 0)
 				break;
+			continue;
+		}
 
 		arg = next_arg( &cmd );
 		if (*arg == '*') {
@@ -820,8 +848,17 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
 				else if (!strcmp( "RECENT", arg1 ))
 					ctx->gen.recent = atoi( arg );
 				else if(!strcmp ( "FETCH", arg1 )) {
-					if (parse_fetch( ctx, cmd ))
+					parse_list_init( &ctx->parse_list_sts );
+				  do_fetch:
+					if ((resp = parse_imap_list( ctx, &cmd, &ctx->parse_list_sts )) == LIST_BAD)
 						break; /* stream is likely to be useless now */
+					if (resp == LIST_PARTIAL) {
+						if (socket_fill( &ctx->conn ) < 0)
+							break;
+						goto do_fetch;
+					}
+					if (parse_fetch( ctx, ctx->parse_list_sts.head ) < 0)
+						break; /* this may mean anything, so prefer not to spam the log */
 				}
 			} else {
 				error( "IMAP error: unrecognized untagged response '%s'\n", arg );

src/isync.h

@@ -79,8 +79,9 @@ typedef struct {
 	SSL *ssl;
 #endif
 
-	int bytes;
+	int offset; /* start of filled bytes in buffer */
-	int offset;
+	int bytes; /* number of filled bytes in buffer */
+	int scanoff; /* offset to continue scanning for newline at, relative to 'offset' */
 	char buf[1024];
 } conn_t;
 
@@ -332,13 +333,13 @@ extern const char *Home;
 int socket_connect( const server_conf_t *conf, conn_t *sock );
 int socket_start_tls( const server_conf_t *conf, conn_t *sock );
 void socket_close( conn_t *sock );
-int socket_read( conn_t *sock, char *buf, int len );
+int socket_fill( conn_t *sock );
+int socket_read( conn_t *sock, char *buf, int len ); /* never waits */
+char *socket_read_line( conn_t *sock ); /* don't free return value; never waits */
 typedef enum { KeepOwn = 0, GiveOwn } ownership_t;
 int socket_write( conn_t *sock, char *buf, int len, ownership_t takeOwn );
 int socket_pending( conn_t *sock );
 
-int buffer_gets( conn_t *b, char **s );
-
 void cram( const char *challenge, const char *user, const char *pass,
            char **_final, int *_finallen );
 

src/socket.c

@@ -354,11 +354,17 @@ socket_close( conn_t *sock )
 }
 
 int
-socket_read( conn_t *sock, char *buf, int len )
+socket_fill( conn_t *sock )
 {
-	int n;
+	char *buf;
-
+	int n = sock->offset + sock->bytes;
+	int len = sizeof(sock->buf) - n;
+	if (!len) {
+		error( "Socket error: receive buffer full. Probably protocol error.\n" );
+		return -1;
+	}
 	assert( sock->fd >= 0 );
+	buf = sock->buf + n;
 	n =
 #ifdef HAVE_LIBSSL
 		sock->ssl ? SSL_read( sock->ssl, buf, len ) :
@@ -368,10 +374,55 @@ socket_read( conn_t *sock, char *buf, int len )
 		socket_perror( "read", sock, n );
 		close( sock->fd );
 		sock->fd = -1;
+		return -1;
+	} else {
+		sock->bytes += n;
+		return 0;
 	}
+}
+
+int
+socket_read( conn_t *conn, char *buf, int len )
+{
+	int n = conn->bytes;
+	if (n > len)
+		n = len;
+	memcpy( buf, conn->buf + conn->offset, n );
+	if (!(conn->bytes -= n))
+		conn->offset = 0;
+	else
+		conn->offset += n;
 	return n;
 }
 
+char *
+socket_read_line( conn_t *b )
+{
+	char *p, *s;
+	int n;
+
+	s = b->buf + b->offset;
+	p = memchr( s + b->scanoff, '\n', b->bytes - b->scanoff );
+	if (!p) {
+		b->scanoff = b->bytes;
+		if (b->offset + b->bytes == sizeof(b->buf)) {
+			memmove( b->buf, b->buf + b->offset, b->bytes );
+			b->offset = 0;
+		}
+		return 0;
+	}
+	n = p + 1 - s;
+	b->offset += n;
+	b->bytes -= n;
+	b->scanoff = 0;
+	if (p != s && p[-1] == '\r')
+		p--;
+	*p = 0;
+	if (DFlags & VERBOSE)
+		puts( s );
+	return s;
+}
+
 int
 socket_write( conn_t *sock, char *buf, int len, ownership_t takeOwn )
 {
@@ -410,57 +461,6 @@ socket_pending( conn_t *sock )
 	return 0;
 }
 
-/* simple line buffering */
-int
-buffer_gets( conn_t *b, char **s )
-{
-	int n;
-	int start = b->offset;
-
-	*s = b->buf + start;
-
-	for (;;) {
-		/* make sure we have enough data to read the \r\n sequence */
-		if (b->offset + 1 >= b->bytes) {
-			if (start) {
-				/* shift down used bytes */
-				*s = b->buf;
-
-				assert( start <= b->bytes );
-				n = b->bytes - start;
-
-				if (n)
-					memmove( b->buf, b->buf + start, n );
-				b->offset -= start;
-				b->bytes = n;
-				start = 0;
-			}
-
-			n = socket_read( b, b->buf + b->bytes,
-			                 sizeof(b->buf) - b->bytes );
-
-			if (n <= 0)
-				return -1;
-
-			b->bytes += n;
-		}
-
-		if (b->buf[b->offset] == '\r') {
-			assert( b->offset + 1 < b->bytes );
-			if (b->buf[b->offset + 1] == '\n') {
-				b->buf[b->offset] = 0;  /* terminate the string */
-				b->offset += 2; /* next line */
-				if (DFlags & VERBOSE)
-					puts( *s );
-				return 0;
-			}
-		}
-
-		b->offset++;
-	}
-	/* not reached */
-}
-
 #ifdef HAVE_LIBSSL
 /* this isn't strictly socket code, but let's have all OpenSSL use in one file. */