From 9bccd31b365e01650db50b42920a72dd613b3327 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Sun, 28 Jun 2015 21:16:48 +0100 Subject: [PATCH] Don't make unnecessary copies of topic in db__messages_store. --- src/database.c | 64 ++++++++++++++++++++++-------------------- src/mosquitto_broker.h | 2 +- src/persist.c | 10 +++---- src/read_handle.c | 5 ++-- 4 files changed, 42 insertions(+), 39 deletions(-) diff --git a/src/database.c b/src/database.c index 0235d443..e17f14de 100644 --- a/src/database.c +++ b/src/database.c @@ -487,30 +487,41 @@ int db__messages_easy_queue(struct mosquitto_db *db, struct mosquitto *context, { struct mosquitto_msg_store *stored; char *source_id; + char *topic_heap; assert(db); if(!topic) return MOSQ_ERR_INVAL; + topic_heap = mosquitto__strdup(topic); + if(!topic_heap) return MOSQ_ERR_INVAL; if(context && context->id){ source_id = context->id; }else{ source_id = ""; } - if(db__message_store(db, source_id, 0, topic, qos, payloadlen, payload, retain, &stored, 0)) return 1; + if(db__message_store(db, source_id, 0, topic_heap, qos, payloadlen, payload, retain, &stored, 0)) return 1; - return sub__messages_queue(db, source_id, topic, qos, retain, &stored); + return sub__messages_queue(db, source_id, topic_heap, qos, retain, &stored); } -int db__message_store(struct mosquitto_db *db, const char *source, uint16_t source_mid, const char *topic, int qos, uint32_t payloadlen, const void *payload, int retain, struct mosquitto_msg_store **stored, dbid_t store_id) +/* This function requires topic to be allocated on the heap. Once called, it owns topic and will free it on error. */ +int db__message_store(struct mosquitto_db *db, const char *source, uint16_t source_mid, char *topic, int qos, uint32_t payloadlen, const void *payload, int retain, struct mosquitto_msg_store **stored, dbid_t store_id) { - struct mosquitto_msg_store *temp; + struct mosquitto_msg_store *temp = NULL; + int rc = MOSQ_ERR_SUCCESS; assert(db); assert(stored); temp = mosquitto__malloc(sizeof(struct mosquitto_msg_store)); - if(!temp) return MOSQ_ERR_NOMEM; + if(!temp){ + rc = MOSQ_ERR_NOMEM; + goto error; + } + + temp->topic = NULL; + temp->payload.ptr = NULL; temp->ref_count = 0; if(source){ @@ -519,46 +530,27 @@ int db__message_store(struct mosquitto_db *db, const char *source, uint16_t sour temp->source_id = mosquitto__strdup(""); } if(!temp->source_id){ - mosquitto__free(temp); log__printf(NULL, MOSQ_LOG_ERR, "Error: Out of memory."); - return MOSQ_ERR_NOMEM; + rc = MOSQ_ERR_NOMEM; + goto error; } temp->source_mid = source_mid; temp->mid = 0; temp->qos = qos; temp->retain = retain; - if(topic){ - temp->topic = mosquitto__strdup(topic); - if(!temp->topic){ - mosquitto__free(temp->source_id); - mosquitto__free(temp); - log__printf(NULL, MOSQ_LOG_ERR, "Error: Out of memory."); - return MOSQ_ERR_NOMEM; - } - }else{ - temp->topic = NULL; - } + temp->topic = topic; + topic = NULL; temp->payloadlen = payloadlen; if(payloadlen){ - UHPA_ALLOC_PAYLOAD(temp); - if(!UHPA_ACCESS_PAYLOAD(temp)){ - if(temp->source_id) mosquitto__free(temp->source_id); - if(temp->topic) mosquitto__free(temp->topic); - mosquitto__free(temp); - return MOSQ_ERR_NOMEM; + if(UHPA_ALLOC_PAYLOAD(temp) == 0){ + rc = MOSQ_ERR_NOMEM; + goto error; } memcpy(UHPA_ACCESS_PAYLOAD(temp), payload, sizeof(char)*payloadlen); }else{ temp->payload.ptr = NULL; } - if(!temp->source_id || (payloadlen && !UHPA_ACCESS_PAYLOAD(temp))){ - if(temp->source_id) mosquitto__free(temp->source_id); - if(temp->topic) mosquitto__free(temp->topic); - UHPA_FREE_PAYLOAD(temp); - mosquitto__free(temp); - return 1; - } temp->dest_ids = NULL; temp->dest_id_count = 0; db->msg_store_count++; @@ -573,6 +565,16 @@ int db__message_store(struct mosquitto_db *db, const char *source, uint16_t sour db__msg_store_add(db, temp); return MOSQ_ERR_SUCCESS; +error: + if(topic){ + mosquitto__free(topic); + } + if(temp){ + if(temp->source_id) mosquitto__free(temp->source_id); + if(temp->topic) mosquitto__free(temp->topic); + mosquitto__free(temp); + } + return rc; } int db__message_store_find(struct mosquitto *context, uint16_t mid, struct mosquitto_msg_store **stored) diff --git a/src/mosquitto_broker.h b/src/mosquitto_broker.h index b0fc8b81..a06c057e 100644 --- a/src/mosquitto_broker.h +++ b/src/mosquitto_broker.h @@ -473,7 +473,7 @@ int db__message_update(struct mosquitto *context, uint16_t mid, enum mosquitto_m int db__message_write(struct mosquitto_db *db, struct mosquitto *context); int db__messages_delete(struct mosquitto_db *db, struct mosquitto *context); int db__messages_easy_queue(struct mosquitto_db *db, struct mosquitto *context, const char *topic, int qos, uint32_t payloadlen, const void *payload, int retain); -int db__message_store(struct mosquitto_db *db, const char *source, uint16_t source_mid, const char *topic, int qos, uint32_t payloadlen, const void *payload, int retain, struct mosquitto_msg_store **stored, dbid_t store_id); +int db__message_store(struct mosquitto_db *db, const char *source, uint16_t source_mid, char *topic, int qos, uint32_t payloadlen, const void *payload, int retain, struct mosquitto_msg_store **stored, dbid_t store_id); int db__message_store_find(struct mosquitto *context, uint16_t mid, struct mosquitto_msg_store **stored); void db__msg_store_add(struct mosquitto_db *db, struct mosquitto_msg_store *store); void db__msg_store_remove(struct mosquitto_db *db, struct mosquitto_msg_store *store); diff --git a/src/persist.c b/src/persist.c index a71d821d..8efde80c 100644 --- a/src/persist.c +++ b/src/persist.c @@ -632,13 +632,13 @@ static int persist__msg_store_chunk_restore(struct mosquitto_db *db, FILE *db_fp rc = db__message_store(db, source_id, source_mid, topic, qos, payloadlen, payload, retain, &stored, store_id); - load->db_id = stored->db_id; - load->store = stored; - - HASH_ADD(hh, db->msg_store_load, db_id, sizeof(dbid_t), load); + if(rc == MOSQ_ERR_SUCCESS){ + load->db_id = stored->db_id; + load->store = stored; + HASH_ADD(hh, db->msg_store_load, db_id, sizeof(dbid_t), load); + } if(source_id) mosquitto__free(source_id); - mosquitto__free(topic); mosquitto__free(payload); return rc; diff --git a/src/read_handle.c b/src/read_handle.c index 18737cc2..55eb3f42 100644 --- a/src/read_handle.c +++ b/src/read_handle.c @@ -217,13 +217,15 @@ int handle__publish(struct mosquitto_db *db, struct mosquitto *context) if(!stored){ dup = 0; if(db__message_store(db, context->id, mid, topic, qos, payloadlen, payload, retain, &stored, 0)){ - mosquitto__free(topic); if(payload) mosquitto__free(payload); return 1; } }else{ + mosquitto__free(topic); + topic = stored->topic; dup = 1; } + switch(qos){ case 0: if(sub__messages_queue(db, context->id, topic, qos, retain, &stored)) rc = 1; @@ -247,7 +249,6 @@ int handle__publish(struct mosquitto_db *db, struct mosquitto *context) } break; } - mosquitto__free(topic); if(payload) mosquitto__free(payload); return rc;