libdaemon: Make buffer handling asymptotically more efficient.

daemons/lvmetad/lvmetad-core.c

@@ -287,7 +287,9 @@ static response pv_list(lvmetad_state *s, request r)
 	struct dm_config_node *cn = NULL, *cn_pvs;
 	struct dm_hash_node *n;
 	const char *id;
-	response res = { .buffer = NULL };
+	response res;
+
+	buffer_init( &res.buffer );
 
 	if (!(res.cft = dm_config_create()))
 		return res; /* FIXME error reporting */
@@ -313,9 +315,11 @@ static response pv_lookup(lvmetad_state *s, request r)
 {
 	const char *pvid = daemon_request_str(r, "uuid", NULL);
 	int64_t devt = daemon_request_int(r, "device", 0);
-	response res = { .buffer = NULL };
+	response res;
 	struct dm_config_node *pv;
 
+	buffer_init( &res.buffer );
+
 	if (!pvid && !devt)
 		return reply_fail("need PVID or device");
 
@@ -355,7 +359,10 @@ static response vg_list(lvmetad_state *s, request r)
 	struct dm_hash_node *n;
 	const char *id;
 	const char *name;
-	response res = { .buffer = NULL };
+	response res;
+
+	buffer_init( &res.buffer );
+
 	if (!(res.cft = dm_config_create()))
                 goto bad; /* FIXME: better error reporting */
 
@@ -422,11 +429,13 @@ static response vg_lookup(lvmetad_state *s, request r)
 {
 	struct dm_config_tree *cft;
 	struct dm_config_node *metadata, *n;
-	response res = { .buffer = NULL };
+	response res;
 
 	const char *uuid = daemon_request_str(r, "uuid", NULL);
 	const char *name = daemon_request_str(r, "name", NULL);
 
+	buffer_init( &res.buffer );
+
 	DEBUG(s, "vg_lookup: uuid = %s, name = %s", uuid, name);
 
 	if (!uuid || !name) {

libdaemon/client/config-util.c

@@ -21,17 +21,13 @@
 #include "config-util.h"
 #include "libdevmapper.h"
 
-char *format_buffer_v(const char *head, va_list ap)
+int buffer_append_vf(struct buffer *buf, va_list ap)
 {
-	char *buffer, *old;
+	char *append, *old;
 	char *next;
 	int keylen;
 
-	dm_asprintf(&buffer, "%s", head);
-	if (!buffer) goto fail;
-
 	while ((next = va_arg(ap, char *))) {
-		old = buffer;
 		if (!strchr(next, '=')) {
 			log_error(INTERNAL_ERROR "Bad format string at '%s'", next);
 			goto fail;
@@ -39,36 +35,34 @@ char *format_buffer_v(const char *head, va_list ap)
 		keylen = strchr(next, '=') - next;
 		if (strstr(next, "%d") || strstr(next, "%" PRId64)) {
 			int64_t value = va_arg(ap, int64_t);
-			dm_asprintf(&buffer, "%s%.*s= %" PRId64 "\n", buffer, keylen, next, value);
-			dm_free(old);
+			dm_asprintf(&append, "%.*s= %" PRId64 "\n", keylen, next, value);
 		} else if (strstr(next, "%s")) {
 			char *value = va_arg(ap, char *);
-			dm_asprintf(&buffer, "%s%.*s= \"%s\"\n", buffer, keylen, next, value);
-			dm_free(old);
+			dm_asprintf(&append, "%.*s= \"%s\"\n", keylen, next, value);
 		} else if (strstr(next, "%b")) {
 			char *block = va_arg(ap, char *);
 			if (!block)
 				continue;
-			dm_asprintf(&buffer, "%s%.*s%s", buffer, keylen, next, block);
-			dm_free(old);
+			dm_asprintf(&append, "%.*s%s", keylen, next, block);
 		} else {
-			dm_asprintf(&buffer, "%s%s", buffer, next);
-			dm_free(old);
+			dm_asprintf(&append, "%s", next);
 		}
-		if (!buffer) goto fail;
+		if (!append) goto fail;
+		buffer_append(buf, append);
+		dm_free(append);
 	}
 
-	return buffer;
+	return 1;
 fail:
-	dm_free(buffer);
-	return NULL;
+	dm_free(append);
+	return 0;
 }
 
-char *format_buffer(const char *head, ...)
+int buffer_append_f(struct buffer *buf, ...)
 {
 	va_list ap;
-	va_start(ap, head);
-	char *res = format_buffer_v(head, ap);
+	va_start(ap, buf);
+	int res = buffer_append_vf(buf, ap);
 	va_end(ap);
 	return res;
 }
@@ -263,26 +257,57 @@ struct dm_config_node *config_make_nodes(struct dm_config_tree *cft,
 	return res;
 }
 
-int buffer_rewrite(char **buf, const char *format, const char *string)
+int buffer_realloc(struct buffer *buf, int needed)
 {
-	char *old = *buf;
-	int r = dm_asprintf(buf, format, *buf, string);
+	char *new;
+	int alloc = buf->allocated;
+	if (alloc < needed)
+		alloc = needed;
 
-	dm_free(old);
+	buf->allocated += alloc;
+	new = realloc(buf->mem, buf->allocated);
+	if (new)
+		buf->mem = new;
+	else { /* utter failure */
+		dm_free(buf->mem);
+		buf->mem = 0;
+		buf->allocated = buf->used = 0;
+		return 0;
+	}
+	return 1;
+}
 
-	return (r < 0) ? 0 : 1;
+int buffer_append(struct buffer *buf, const char *string)
+{
+	int len = strlen(string);
+	char *new;
+
+	if (buf->allocated - buf->used <= len)
+		buffer_realloc(buf, len + 1);
+
+	strcpy(buf->mem + buf->used, string);
+	buf->used += len;
+	return 1;
 }
 
 int buffer_line(const char *line, void *baton)
 {
-	char **buffer = baton;
-
-	if (*buffer) {
-		if (!buffer_rewrite(buffer, "%s\n%s", line))
-			return 0;
-	} else if (dm_asprintf(buffer, "%s\n", line) < 0)
+	struct buffer *buf = baton;
+	if (!buffer_append(buf, line))
+		return 0;
+	if (!buffer_append(buf, "\n"))
 		return 0;
-
 	return 1;
 }
 
+void buffer_destroy(struct buffer *buf)
+{
+	dm_free(buf->mem);
+	buffer_init(buf);
+}
+
+void buffer_init(struct buffer *buf)
+{
+	buf->allocated = buf->used = 0;
+	buf->mem = 0;
+}

libdaemon/client/config-util.h

@@ -20,11 +20,20 @@
 
 #include <stdarg.h>
 
-char *format_buffer_v(const char *head, va_list ap);
-char *format_buffer(const char *head, ...);
+struct buffer {
+	int allocated;
+	int used;
+	char *mem;
+};
+
+int buffer_append_vf(struct buffer *buf, va_list ap);
+int buffer_append_f(struct buffer *buf, ...);
+int buffer_append(struct buffer *buf, const char *string);
+void buffer_init(struct buffer *buf);
+void buffer_destroy(struct buffer *buf);
+int buffer_realloc(struct buffer *buf, int required);
 
 int buffer_line(const char *line, void *baton);
-int buffer_rewrite(char **buf, const char *format, const char *string);
 
 int set_flag(struct dm_config_tree *cft, struct dm_config_node *parent,
 	     const char *field, const char *flag, int want);

libdaemon/client/daemon-client.c

@@ -73,24 +73,24 @@ daemon_reply daemon_send(daemon_handle h, daemon_request rq)
 {
 	daemon_reply reply = { .cft = NULL, .error = 0 };
 	assert(h.socket_fd >= 0);
-	char *buffer = rq.buffer;
+	struct buffer buffer = rq.buffer;
 
-	if (!buffer)
+	if (!buffer.mem)
 		dm_config_write_node(rq.cft->root, buffer_line, &buffer);
 
-	assert(buffer);
-	if (!write_buffer(h.socket_fd, buffer, strlen(buffer)))
+	assert(buffer.mem);
+	if (!buffer_write(h.socket_fd, &buffer))
 		reply.error = errno;
 
-	if (read_buffer(h.socket_fd, &reply.buffer)) {
-		reply.cft = dm_config_from_string(reply.buffer);
+	if (buffer_read(h.socket_fd, &reply.buffer)) {
+		reply.cft = dm_config_from_string(reply.buffer.mem);
 		if (!reply.cft)
 			reply.error = EPROTO;
 	} else
 		reply.error = errno;
 
-	if (buffer != rq.buffer)
-		dm_free(buffer);
+	if (buffer.mem != rq.buffer.mem)
+		buffer_destroy(&buffer);
 
 	return reply;
 }
@@ -98,28 +98,22 @@ daemon_reply daemon_send(daemon_handle h, daemon_request rq)
 void daemon_reply_destroy(daemon_reply r) {
 	if (r.cft)
 		dm_config_destroy(r.cft);
-	dm_free(r.buffer);
+	buffer_destroy(&r.buffer);
 }
 
 daemon_reply daemon_send_simple_v(daemon_handle h, const char *id, va_list ap)
 {
-	static const daemon_reply err = { .error = ENOMEM, .buffer = NULL, .cft = NULL };
+	static const daemon_reply err = { .error = ENOMEM, .buffer = { 0, 0, NULL }, .cft = NULL };
 	daemon_request rq = { .cft = NULL };
 	daemon_reply repl;
-	char *rq_line;
 
-	rq_line = format_buffer("", "request = %s", id, NULL);
-	if (!rq_line)
+	if (!buffer_append_f(&rq.buffer, "request = %s", id, NULL))
 		return err;
-
-	rq.buffer = format_buffer_v(rq_line, ap);
-	dm_free(rq_line);
-
-	if (!rq.buffer)
+	if (!buffer_append_vf(&rq.buffer, ap))
 		return err;
 
 	repl = daemon_send(h, rq);
-	dm_free(rq.buffer);
+	buffer_destroy(&rq.buffer);
 
 	return repl;
 }
@@ -142,7 +136,7 @@ daemon_request daemon_request_make(const char *id)
 {
 	daemon_request r;
 	r.cft = NULL;
-	r.buffer = NULL;
+	buffer_init(&r.buffer);
 
 	if (!(r.cft = dm_config_create()))
 		goto bad;
@@ -181,6 +175,6 @@ int daemon_request_extend(daemon_request r, ...)
 void daemon_request_destroy(daemon_request r) {
 	if (r.cft)
 		dm_config_destroy(r.cft);
-	dm_free(r.buffer);
+	buffer_destroy(&r.buffer);
 }
 

libdaemon/client/daemon-client.h

@@ -16,6 +16,7 @@
 #define _LVM_DAEMON_COMMON_CLIENT_H
 
 #include "libdevmapper.h"
+#include "config-util.h"
 
 typedef struct {
 	int socket_fd; /* the fd we use to talk to the daemon */
@@ -38,7 +39,7 @@ typedef struct {
 } daemon_info;
 
 typedef struct {
-	char *buffer;
+	struct buffer buffer;
 	/*
 	 * The request looks like this:
 	 *    request = "id"
@@ -55,7 +56,7 @@ typedef struct {
 
 typedef struct {
 	int error; /* 0 for success */
-	char *buffer; /* textual reply */
+	struct buffer buffer;
 	struct dm_config_tree *cft; /* parsed reply, if available */
 } daemon_reply;
 

libdaemon/client/daemon-io.c

@@ -29,26 +29,22 @@
  *
  * See also write_buffer about blocking (read_buffer has identical behaviour).
  */
-int read_buffer(int fd, char **buffer) {
-	int bytes = 0;
-	int buffersize = 32;
-	char *new;
-	*buffer = dm_malloc(buffersize + 1);
+int buffer_read(int fd, struct buffer *buffer) {
+	if (!buffer_realloc(buffer, 32)) /* ensure we have some space */
+		goto fail;
 
 	while (1) {
-		int result = read(fd, (*buffer) + bytes, buffersize - bytes);
+		int result = read(fd, buffer->mem + buffer->used, buffer->allocated - buffer->used);
 		if (result > 0) {
-			bytes += result;
-			if (!strncmp((*buffer) + bytes - 4, "\n##\n", 4)) {
-				*(*buffer + bytes - 4) = 0;
+			buffer->used += result;
+			if (!strncmp((buffer->mem) + buffer->used - 4, "\n##\n", 4)) {
+				*(buffer->mem + buffer->used - 4) = 0;
+				buffer->used -= 4;
 				break; /* success, we have the full message now */
 			}
-			if (bytes == buffersize) {
-				buffersize += 1024;
-				if (!(new = realloc(*buffer, buffersize + 1)))
+			if (buffer->used - buffer->allocated < 32)
+				if (!buffer_realloc(buffer, 1024))
 					goto fail;
-				*buffer = new;
-			}
 			continue;
 		}
 		if (result == 0) {
@@ -61,8 +57,6 @@ int read_buffer(int fd, char **buffer) {
 	}
 	return 1;
 fail:
-	dm_free(*buffer);
-	*buffer = NULL;
 	return 0;
 }
 
@@ -72,18 +66,19 @@ fail:
  *
  * TODO use select on EWOULDBLOCK/EAGAIN/EINTR to avoid useless spinning
  */
-int write_buffer(int fd, const char *buffer, int length) {
-	static const char terminate[] = "\n##\n";
+int buffer_write(int fd, struct buffer *buffer) {
+	struct buffer terminate = { .mem = (char *) "\n##\n", .used = 4 };
 	int done = 0;
 	int written = 0;
+	struct buffer *use = buffer;
 write:
 	while (1) {
-		int result = write(fd, buffer + written, length - written);
+		int result = write(fd, use->mem + written, use->used - written);
 		if (result > 0)
 			written += result;
 		if (result < 0 && errno != EWOULDBLOCK && errno != EAGAIN && errno != EINTR)
 			return 0; /* too bad */
-		if (written == length) {
+		if (written == use->used) {
 			if (done)
 				return 1;
 			else
@@ -91,8 +86,7 @@ write:
 		}
 	}
 
-	buffer = terminate;
-	length = 4;
+	use = &terminate;
 	written = 0;
 	done = 1;
 	goto write;

libdaemon/client/daemon-io.h

@@ -16,6 +16,7 @@
 #define _LVM_DAEMON_IO_H
 
 #include "configure.h"
+#include "config-util.h"
 #include "libdevmapper.h"
 
 #define _REENTRANT
@@ -24,7 +25,7 @@
 
 /* TODO function names */
 
-int read_buffer(int fd, char **buffer);
-int write_buffer(int fd, const char *buffer, int length);
+int buffer_read(int fd, struct buffer *buffer);
+int buffer_write(int fd, struct buffer *buffer);
 
 #endif /* _LVM_DAEMON_SHARED_H */

libdaemon/server/daemon-server.c

@@ -330,22 +330,20 @@ response daemon_reply_simple(const char *id, ...)
 {
 	va_list ap;
 	response res = { .cft = NULL };
-	char *res_line = NULL;
 
 	va_start(ap, id);
 
-	if (!(res_line = format_buffer("", "response = %s", id, NULL))) {
+	buffer_init(&res.buffer);
+	if (!buffer_append_f(&res.buffer, "response = %s", id, NULL)) {
 		res.error = ENOMEM;
 		goto end;
 	}
-
-	if (!(res.buffer = format_buffer_v(res_line, ap))) {
+	if (!buffer_append_vf(&res.buffer, ap)) {
 		res.error = ENOMEM;
 		goto end;
 	}
 
 end:
-	dm_free(res_line);
 	va_end(ap);
 	return res;
 }
@@ -364,24 +362,27 @@ static response builtin_handler(daemon_state s, client_handle h, request r)
 					   "version = %" PRId64, (int64_t) s.protocol_version, NULL);
 	}
 
-	response res = { .buffer = NULL, .error = EPROTO };
+	response res = { .error = EPROTO };
+	buffer_init(&res.buffer);
 	return res;
 }
 
 static void *client_thread(void *baton)
 {
 	struct thread_baton *b = baton;
-	request req = { .buffer = NULL };
+	request req;
 	response res;
 
+	buffer_init(&req.buffer);
+
 	while (1) {
-		if (!read_buffer(b->client.socket_fd, &req.buffer))
+		if (!buffer_read(b->client.socket_fd, &req.buffer))
 			goto fail;
 
-		req.cft = dm_config_from_string(req.buffer);
+		req.cft = dm_config_from_string(req.buffer.mem);
 
 		if (!req.cft)
-			fprintf(stderr, "error parsing request:\n %s\n", req.buffer);
+			fprintf(stderr, "error parsing request:\n %s\n", req.buffer.mem);
 		else
 			daemon_log_cft(b->s.log, DAEMON_LOG_WIRE, "<- ", req.cft->root);
 
@@ -390,27 +391,27 @@ static void *client_thread(void *baton)
 		if (res.error == EPROTO) /* Not a builtin, delegate to the custom handler. */
 			res = b->s.handler(b->s, b->client, req);
 
-		if (!res.buffer) {
+		if (!res.buffer.mem) {
 			dm_config_write_node(res.cft->root, buffer_line, &res.buffer);
-			if (!buffer_rewrite(&res.buffer, "%s\n\n", NULL))
+			if (!buffer_append(&res.buffer, "\n\n"))
 				goto fail;
 			dm_config_destroy(res.cft);
 		}
 
 		if (req.cft)
 			dm_config_destroy(req.cft);
-		dm_free(req.buffer);
+		buffer_destroy(&req.buffer);
 
-		daemon_log_multi(b->s.log, DAEMON_LOG_WIRE, "-> ", res.buffer);
-		write_buffer(b->client.socket_fd, res.buffer, strlen(res.buffer));
+		daemon_log_multi(b->s.log, DAEMON_LOG_WIRE, "-> ", res.buffer.mem);
+		buffer_write(b->client.socket_fd, &res.buffer);
 
-		free(res.buffer);
+		buffer_destroy(&res.buffer);
 	}
 fail:
 	/* TODO what should we really do here? */
 	if (close(b->client.socket_fd))
 		perror("close");
-	dm_free(req.buffer);
+	buffer_destroy(&req.buffer);
 	dm_free(baton);
 	return NULL;
 }

libdaemon/server/daemon-server.h

@@ -26,13 +26,13 @@ typedef struct {
 
 typedef struct {
 	struct dm_config_tree *cft;
-	char *buffer;
+	struct buffer buffer;
 } request;
 
 typedef struct {
 	int error;
 	struct dm_config_tree *cft;
-	char *buffer;
+	struct buffer buffer;
 } response;
 
 struct daemon_state;