]> sourceware.org Git - lvm2.git/commitdiff
libdaemon: Make buffer handling asymptotically more efficient.
authorPetr Rockai <prockai@redhat.com>
Thu, 11 Oct 2012 12:17:17 +0000 (14:17 +0200)
committerPetr Rockai <prockai@redhat.com>
Thu, 11 Oct 2012 16:09:41 +0000 (18:09 +0200)
daemons/lvmetad/lvmetad-core.c
libdaemon/client/config-util.c
libdaemon/client/config-util.h
libdaemon/client/daemon-client.c
libdaemon/client/daemon-client.h
libdaemon/client/daemon-io.c
libdaemon/client/daemon-io.h
libdaemon/server/daemon-server.c
libdaemon/server/daemon-server.h

index 8a1378aa62f1a16132fe0ed4931f0fe775d7217f..47b07aaa86899d5e7c58acd95bbed50953218e2f 100644 (file)
@@ -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) {
index 25731398075913e24a874bcf8e9b6bf70ec13977..30fc4f86bc15017799e81f89e7b720212f96cb7c 100644 (file)
 #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;
+
+       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;
+}
+
+int buffer_append(struct buffer *buf, const char *string)
+{
+       int len = strlen(string);
+       char *new;
 
-       dm_free(old);
+       if (buf->allocated - buf->used <= len)
+               buffer_realloc(buf, len + 1);
 
-       return (r < 0) ? 0 : 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;
+}
index ae5e5569f605d82e6162a39db51e2b3b44422030..f03bcabafe5e4703a1876edf24aaa329e8efbb38 100644 (file)
 
 #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);
index c87856c0adfa936be0ed37d3cc43c22cb998d54f..107882b48ef02767446ef47e70c965a0061e557c 100644 (file)
@@ -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);
 }
 
index 2863d03880afd01986513327246d865969d35ac4..6ba65e6c725e7c864a18a0be36c682f02764e52b 100644 (file)
@@ -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;
 
index 4af9343f830e0804501c236ff75328d2557168be..50ef9b26282ab5908cb32e07037eb1c805aae596 100644 (file)
  *
  * 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;
index e6e5f0625ea01295262315d1a40aea6e336a0204..1f55af7ae47d738b3af65e5b8241af1e35a1a5ca 100644 (file)
@@ -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 */
index 9dd911f52211069183c99d1b6136afe59ae7aeb3..051e7c21b47864fd8fab87923329b98c2b9f49df 100644 (file)
@@ -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;
 }
index df7ed8e056e7961d751c326d2c2a2cc4a6d32ca5..210960231c3dfe28627e36a8fba232603df89b22 100644 (file)
@@ -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;
This page took 0.064941 seconds and 5 git commands to generate.