diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c index 8fca8a850a..bb39676e8b 100644 --- a/tools/xenstore/xenstored_core.c +++ b/tools/xenstore/xenstored_core.c @@ -647,17 +647,20 @@ void send_reply(struct connection *conn, enum xsd_sockmsg_type type, return; } - /* Message is a child of the connection context for auto-cleanup. */ - bdata = new_buffer(conn); - bdata->buffer = talloc_array(bdata, char, len); - - /* Echo request header in reply unless this is an async watch event. */ + /* Replies reuse the request buffer, events need a new one. */ if (type != XS_WATCH_EVENT) { - memcpy(&bdata->hdr.msg, &conn->in->hdr.msg, - sizeof(struct xsd_sockmsg)); + bdata = conn->in; + bdata->inhdr = true; + bdata->used = 0; + conn->in = NULL; } else { - memset(&bdata->hdr.msg, 0, sizeof(struct xsd_sockmsg)); + /* Message is a child of the connection for auto-cleanup. */ + bdata = new_buffer(conn); } + if (len <= DEFAULT_BUFFER_SIZE) + bdata->buffer = bdata->default_buffer; + else + bdata->buffer = talloc_array(bdata, char, len); /* Update relevant header fields and fill in the message body. */ bdata->hdr.msg.type = type; @@ -733,7 +736,7 @@ static char *perms_to_strings(const void *ctx, return strings; } -char *canonicalize(struct connection *conn, const char *node) +char *canonicalize(struct connection *conn, const void *ctx, const char *node) { const char *prefix; @@ -741,7 +744,7 @@ char *canonicalize(struct connection *conn, const char *node) return (char *)node; prefix = get_implicit_path(conn); if (prefix) - return talloc_asprintf(node, "%s/%s", prefix, node); + return talloc_asprintf(ctx, "%s/%s", prefix, node); return (char *)node; } @@ -755,7 +758,7 @@ static struct node *get_node_canonicalized(struct connection *conn, if (!canonical_name) canonical_name = &tmp_name; - *canonical_name = canonicalize(conn, name); + *canonical_name = canonicalize(conn, ctx, name); return get_node(conn, ctx, *canonical_name, perm); } @@ -865,17 +868,18 @@ static char *basename(const char *name) return strrchr(name, '/') + 1; } -static struct node *construct_node(struct connection *conn, const char *name) +static struct node *construct_node(struct connection *conn, const void *ctx, + const char *name) { const char *base; unsigned int baselen; struct node *parent, *node; - char *children, *parentname = get_parent(name, name); + char *children, *parentname = get_parent(ctx, name); /* If parent doesn't exist, create it. */ parent = read_node(conn, parentname, parentname); if (!parent) - parent = construct_node(conn, parentname); + parent = construct_node(conn, ctx, parentname); if (!parent) return NULL; @@ -885,14 +889,14 @@ static struct node *construct_node(struct connection *conn, const char *name) /* Add child to parent. */ base = basename(name); baselen = strlen(base) + 1; - children = talloc_array(name, char, parent->childlen + baselen); + children = talloc_array(ctx, char, parent->childlen + baselen); memcpy(children, parent->children, parent->childlen); memcpy(children + parent->childlen, base, baselen); parent->children = children; parent->childlen += baselen; /* Allocate node */ - node = talloc(name, struct node); + node = talloc(ctx, struct node); node->tdb = tdb_context(conn); node->name = talloc_strdup(node, name); @@ -926,13 +930,13 @@ static int destroy_node(void *_node) return 0; } -static struct node *create_node(struct connection *conn, +static struct node *create_node(struct connection *conn, const void *ctx, const char *name, void *data, unsigned int datalen) { struct node *node, *i; - node = construct_node(conn, name); + node = construct_node(conn, ctx, name); if (!node) return NULL; @@ -975,7 +979,8 @@ static int do_write(struct connection *conn, struct buffered_data *in) /* No permissions, invalid input? */ if (errno != ENOENT) return errno; - node = create_node(conn, name, in->buffer + offset, datalen); + node = create_node(conn, in, name, in->buffer + offset, + datalen); if (!node) return errno; } else { @@ -1004,7 +1009,7 @@ static int do_mkdir(struct connection *conn, struct buffered_data *in) /* No permissions? */ if (errno != ENOENT) return errno; - node = create_node(conn, name, NULL, 0); + node = create_node(conn, in, name, NULL, 0); if (!node) return errno; fire_watches(conn, in, name, false); @@ -1075,12 +1080,13 @@ static bool delete_child(struct connection *conn, } -static int _rm(struct connection *conn, struct node *node, const char *name) +static int _rm(struct connection *conn, const void *ctx, struct node *node, + const char *name) { /* Delete from parent first, then if we crash, the worst that can happen is the child will continue to take up space, but will otherwise be unreachable. */ - struct node *parent = read_node(conn, name, get_parent(name, name)); + struct node *parent = read_node(conn, ctx, get_parent(ctx, name)); if (!parent) return EINVAL; @@ -1097,7 +1103,7 @@ static void internal_rm(const char *name) char *tname = talloc_strdup(NULL, name); struct node *node = read_node(NULL, tname, tname); if (node) - _rm(NULL, node, tname); + _rm(NULL, tname, node, tname); talloc_free(node); talloc_free(tname); } @@ -1128,7 +1134,7 @@ static int do_rm(struct connection *conn, struct buffered_data *in) if (streq(name, "/")) return EINVAL; - ret = _rm(conn, node, name); + ret = _rm(conn, in, node, name); if (ret) return ret; @@ -1301,8 +1307,7 @@ static void consider_message(struct connection *conn) process_message(conn, conn->in); - talloc_free(conn->in); - conn->in = new_buffer(conn); + assert(conn->in == NULL); } /* Errors in reading or allocating here mean we get out of sync, so we @@ -1310,7 +1315,15 @@ static void consider_message(struct connection *conn) static void handle_input(struct connection *conn) { int bytes; - struct buffered_data *in = conn->in; + struct buffered_data *in; + + if (!conn->in) { + conn->in = new_buffer(conn); + /* In case of no memory just try it next time again. */ + if (!conn->in) + return; + } + in = conn->in; /* Not finished header yet? */ if (in->inhdr) { @@ -1328,7 +1341,10 @@ static void handle_input(struct connection *conn) goto bad_client; } - in->buffer = talloc_array(in, char, in->hdr.msg.len); + if (in->hdr.msg.len <= DEFAULT_BUFFER_SIZE) + in->buffer = in->default_buffer; + else + in->buffer = talloc_array(in, char, in->hdr.msg.len); if (!in->buffer) goto bad_client; in->used = 0; @@ -1377,12 +1393,6 @@ struct connection *new_connection(connwritefn_t *write, connreadfn_t *read) INIT_LIST_HEAD(&new->watches); INIT_LIST_HEAD(&new->transaction_list); - new->in = new_buffer(new); - if (new->in == NULL) { - talloc_free(new); - return NULL; - } - list_add_tail(&new->list, &connections); talloc_set_destructor(new, destroy_conn); trace_create(new, "connection"); @@ -1522,7 +1532,7 @@ static void setup_structure(void) if (remove_local) { internal_rm("/local"); - create_node(NULL, tlocal, NULL, 0); + create_node(NULL, NULL, tlocal, NULL, 0); check_store(); } diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h index 3872dabed9..f6a56f701c 100644 --- a/tools/xenstore/xenstored_core.h +++ b/tools/xenstore/xenstored_core.h @@ -33,6 +33,9 @@ #include "list.h" #include "tdb.h" +/* DEFAULT_BUFFER_SIZE should be large enough for each errno string. */ +#define DEFAULT_BUFFER_SIZE 16 + struct buffered_data { struct list_head list; @@ -50,6 +53,7 @@ struct buffered_data /* The actual data. */ char *buffer; + char default_buffer[DEFAULT_BUFFER_SIZE]; }; struct connection; @@ -139,7 +143,7 @@ void send_reply(struct connection *conn, enum xsd_sockmsg_type type, void send_ack(struct connection *conn, enum xsd_sockmsg_type type); /* Canonicalize this path if possible. */ -char *canonicalize(struct connection *conn, const char *node); +char *canonicalize(struct connection *conn, const void *ctx, const char *node); /* Get this node, checking we have permissions. */ struct node *get_node(struct connection *conn, diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c index 2443b08b64..fefed5eec0 100644 --- a/tools/xenstore/xenstored_domain.c +++ b/tools/xenstore/xenstored_domain.c @@ -329,9 +329,7 @@ static void domain_conn_reset(struct domain *domain) talloc_free(out); } - talloc_free(conn->in->buffer); - memset(conn->in, 0, sizeof(*conn->in)); - conn->in->inhdr = true; + talloc_free(conn->in); domain->interface->req_cons = domain->interface->req_prod = 0; domain->interface->rsp_cons = domain->interface->rsp_prod = 0; diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c index 423fd3e1bc..d314cd516f 100644 --- a/tools/xenstore/xenstored_transaction.c +++ b/tools/xenstore/xenstored_transaction.c @@ -209,8 +209,8 @@ int do_transaction_end(struct connection *conn, struct buffered_data *in) list_del(&trans->list); conn->transaction_started--; - /* Attach transaction to arg for auto-cleanup */ - talloc_steal(arg, trans); + /* Attach transaction to in for auto-cleanup */ + talloc_steal(in, trans); if (streq(arg, "T")) { /* FIXME: Merge, rather failing on any change. */ diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c index e1146edeb2..94251dba91 100644 --- a/tools/xenstore/xenstored_watch.c +++ b/tools/xenstore/xenstored_watch.c @@ -164,7 +164,7 @@ int do_watch(struct connection *conn, struct buffered_data *in) /* check if valid event */ } else { relative = !strstarts(vec[0], "/"); - vec[0] = canonicalize(conn, vec[0]); + vec[0] = canonicalize(conn, in, vec[0]); if (!is_valid_nodename(vec[0])) return EINVAL; } @@ -209,7 +209,7 @@ int do_unwatch(struct connection *conn, struct buffered_data *in) if (get_strings(in, vec, ARRAY_SIZE(vec)) != ARRAY_SIZE(vec)) return EINVAL; - node = canonicalize(conn, vec[0]); + node = canonicalize(conn, in, vec[0]); list_for_each_entry(watch, &conn->watches, list) { if (streq(watch->node, node) && streq(watch->token, vec[1])) { list_del(&watch->list);