From 738a68fd9f569d213f9cae57dd94ff3633aaf7bf Mon Sep 17 00:00:00 2001 From: kanoi Date: Tue, 2 Dec 2014 08:35:00 +1100 Subject: [PATCH] ckdb - fix optioncontrol memory - and add FREENULL to free and null if needed --- src/ckdb.c | 64 ++++++++++++++++++++----------------------------- src/ckdb.h | 9 ++++++- src/ckdb_cmd.c | 25 ++++++++----------- src/ckdb_dbio.c | 46 +++++++++++++---------------------- 4 files changed, 61 insertions(+), 83 deletions(-) diff --git a/src/ckdb.c b/src/ckdb.c index dd534d8d..9a10cb35 100644 --- a/src/ckdb.c +++ b/src/ckdb.c @@ -1070,9 +1070,9 @@ static void free_workinfo_data(K_ITEM *item) DATA_WORKINFO(workinfo, item); if (workinfo->transactiontree) - free(workinfo->transactiontree); + FREENULL(workinfo->transactiontree); if (workinfo->merklehash) - free(workinfo->merklehash); + FREENULL(workinfo->merklehash); } static void free_sharesummary_data(K_ITEM *item) @@ -1082,8 +1082,7 @@ static void free_sharesummary_data(K_ITEM *item) DATA_SHARESUMMARY(sharesummary, item); if (sharesummary->workername) { LIST_MEM_SUB(sharesummary_free, sharesummary->workername); - free(sharesummary->workername); - sharesummary->workername = NULL; + FREENULL(sharesummary->workername); } SET_CREATEBY(sharesummary_free, sharesummary->createby, EMPTY); SET_CREATECODE(sharesummary_free, sharesummary->createcode, EMPTY); @@ -1099,7 +1098,7 @@ static void free_optioncontrol_data(K_ITEM *item) DATA_OPTIONCONTROL(optioncontrol, item); if (optioncontrol->optionvalue) - free(optioncontrol->optionvalue); + FREENULL(optioncontrol->optionvalue); } static void free_markersummary_data(K_ITEM *item) @@ -1108,7 +1107,7 @@ static void free_markersummary_data(K_ITEM *item) DATA_MARKERSUMMARY(markersummary, item); if (markersummary->workername) - free(markersummary->workername); + FREENULL(markersummary->workername); SET_CREATEBY(markersummary_free, markersummary->createby, EMPTY); SET_CREATECODE(markersummary_free, markersummary->createcode, EMPTY); SET_CREATEINET(markersummary_free, markersummary->createinet, EMPTY); @@ -1123,9 +1122,9 @@ static void free_workmarkers_data(K_ITEM *item) DATA_WORKMARKERS(workmarkers, item); if (workmarkers->poolinstance) - free(workmarkers->poolinstance); + FREENULL(workmarkers->poolinstance); if (workmarkers->description) - free(workmarkers->description); + FREENULL(workmarkers->description); } static void free_marks_data(K_ITEM *item) @@ -1134,11 +1133,11 @@ static void free_marks_data(K_ITEM *item) DATA_MARKS(marks, item); if (marks->poolinstance && marks->poolinstance != EMPTY) - free(marks->poolinstance); + FREENULL(marks->poolinstance); if (marks->description && marks->description != EMPTY) - free(marks->description); + FREENULL(marks->description); if (marks->extra && marks->extra != EMPTY) - free(marks->extra); + FREENULL(marks->extra); } #define FREE_TREE(_tree) \ @@ -1473,7 +1472,7 @@ static enum cmd_values breakdown(K_TREE **trf_root, K_STORE **trf_store, STRNCPY(transfer->name, json_key); if (!ok || find_in_ktree(*trf_root, item, cmp_transfer, ctx)) { if (transfer->mvalue != transfer->svalue) - free(transfer->mvalue); + FREENULL(transfer->mvalue); k_add_head(transfer_free, item); } else { *trf_root = add_to_ktree(*trf_root, item, cmp_transfer); @@ -1505,7 +1504,7 @@ static enum cmd_values breakdown(K_TREE **trf_root, K_STORE **trf_store, if (find_in_ktree(*trf_root, item, cmp_transfer, ctx)) { if (transfer->mvalue != transfer->svalue) - free(transfer->mvalue); + FREENULL(transfer->mvalue); k_add_head(transfer_free, item); } else { *trf_root = add_to_ktree(*trf_root, item, cmp_transfer); @@ -2038,7 +2037,7 @@ static void *logger(__maybe_unused void *arg) while (lq_item) { DATA_LOGQUEUE(lq, lq_item); LOGFILE(lq->msg); - free(lq->msg); + FREENULL(lq->msg); K_WLOCK(logqueue_free); k_add_head(logqueue_free, lq_item); @@ -2064,7 +2063,7 @@ static void *logger(__maybe_unused void *arg) while (lq_item) { DATA_LOGQUEUE(lq, lq_item); LOGFILE(lq->msg); - free(lq->msg); + FREENULL(lq->msg); count--; setnow(&now); if ((now.tv_sec - then.tv_sec) > 10) { @@ -2330,8 +2329,7 @@ static void *socketer(__maybe_unused void *arg) rep = malloc(siz); snprintf(rep, siz, "%s.%ld.%s", id, now.tv_sec, ans); send_unix_msg(sockd, rep); - free(ans); - ans = NULL; + FREENULL(ans); switch (cmdnum) { case CMD_AUTH: STORELASTREPLY(auth); @@ -2395,8 +2393,7 @@ static void *socketer(__maybe_unused void *arg) rep = malloc(siz); snprintf(rep, siz, "%s.%ld.%s", id, now.tv_sec, ans); send_unix_msg(sockd, rep); - free(ans); - ans = NULL; + FREENULL(ans); if (cmdnum == CMD_DSP) free(rep); else { @@ -2429,10 +2426,8 @@ static void *socketer(__maybe_unused void *arg) rep = malloc(siz); snprintf(rep, siz, "%s.%ld.%s", id, now.tv_sec, ans); send_unix_msg(sockd, rep); - free(ans); - ans = NULL; - free(rep); - rep = NULL; + FREENULL(ans); + FREENULL(rep); } break; // Always queue (ok.queued) @@ -2504,7 +2499,7 @@ static void *socketer(__maybe_unused void *arg) while (item) { DATA_TRANSFER(transfer, item); if (transfer->mvalue != transfer->svalue) - free(transfer->mvalue); + FREENULL(transfer->mvalue); item = item->next; } K_WLOCK(transfer_free); @@ -2632,7 +2627,7 @@ static bool reload_line(PGconn *conn, char *filename, uint64_t count, char *buf) while (item) { DATA_TRANSFER(transfer, item); if (transfer->mvalue != transfer->svalue) - free(transfer->mvalue); + FREENULL(transfer->mvalue); item = item->next; } K_WLOCK(transfer_free); @@ -2728,8 +2723,7 @@ static bool reload_from(tv_t *start) fp = fopen(filename, "re"); if (!fp) { missingfirst = strdup(filename); - free(filename); - filename = NULL; + FREENULL(filename); errno = 0; missing_count = 1; setnow(&now); @@ -2753,19 +2747,16 @@ static bool reload_from(tv_t *start) if (missing_count++ > 1) free(missinglast); missinglast = strdup(filename); - free(filename); - filename = NULL; + FREENULL(filename); } if (missing_count == 1) LOGWARNING("%s(): skipped %s", __func__, missingfirst+rflen); else { LOGWARNING("%s(): skipped %d files from %s to %s", __func__, missing_count, missingfirst+rflen, missinglast+rflen); - free(missinglast); - missinglast = NULL; + FREENULL(missinglast); } - free(missingfirst); - missingfirst = NULL; + FREENULL(missingfirst); } } @@ -2793,10 +2784,7 @@ static bool reload_from(tv_t *start) } reloading = false; - - free(reload_buf); - reload_buf = NULL; - + FREENULL(reload_buf); return ret; } @@ -2828,7 +2816,7 @@ static void process_queued(PGconn *conn, K_ITEM *wq_item) while (item) { DATA_TRANSFER(transfer, item); if (transfer->mvalue != transfer->svalue) - free(transfer->mvalue); + FREENULL(transfer->mvalue); item = item->next; } K_WLOCK(transfer_free); diff --git a/src/ckdb.h b/src/ckdb.h index ab409fc0..20bd1c2c 100644 --- a/src/ckdb.h +++ b/src/ckdb.h @@ -52,7 +52,7 @@ #define DB_VLOCK "1" #define DB_VERSION "0.9.6" -#define CKDB_VERSION DB_VERSION"-0.701" +#define CKDB_VERSION DB_VERSION"-0.710" #define WHERE_FFL " - from %s %s() line %d" #define WHERE_FFL_HERE __FILE__, __func__, __LINE__ @@ -64,6 +64,13 @@ #define STRINT(x) STRINT2(x) #define STRINT2(x) #x +#define FREENULL(mem) do { \ + if (mem) { \ + free(mem); \ + mem = NULL; \ + } \ + } while (0) + // So they can fit into a 1 byte flag field #define TRUE_STR "Y" #define FALSE_STR "N" diff --git a/src/ckdb_cmd.c b/src/ckdb_cmd.c index 35da99d4..0d443b31 100644 --- a/src/ckdb_cmd.c +++ b/src/ckdb_cmd.c @@ -2758,7 +2758,7 @@ static char *cmd_setopts(PGconn *conn, char *cmd, char *id, ExecStatusType rescode; PGresult *res; bool conned = false; - K_ITEM *t_item, *oc_item = NULL; + K_ITEM *t_item, *oc_item = NULL, *ok = NULL; K_TREE_CTX ctx[1]; char reply[1024] = ""; size_t siz = sizeof(reply); @@ -2806,10 +2806,11 @@ static char *cmd_setopts(PGconn *conn, char *cmd, char *id, } begun = true; } - if (optioncontrol_item_add(conn, oc_item, now, begun)) { - oc_item = NULL; + ok = optioncontrol_item_add(conn, oc_item, now, begun); + oc_item = NULL; + if (ok) db++; - } else { + else { reason = "DBERR"; goto rollback; } @@ -2863,11 +2864,14 @@ static char *cmd_setopts(PGconn *conn, char *cmd, char *id, } begun = true; } - if (!optioncontrol_item_add(conn, oc_item, now, begun)) { + ok = optioncontrol_item_add(conn, oc_item, now, begun); + oc_item = NULL; + if (ok) + db++; + else { reason = "DBERR"; goto rollback; } - db++; } rollback: if (begun) { @@ -2882,15 +2886,6 @@ rollback: if (conned) PQfinish(conn); if (reason) { - if (oc_item) { - if (optioncontrol->optionvalue) { - free(optioncontrol->optionvalue); - optioncontrol->optionvalue = NULL; - } - K_WLOCK(optioncontrol_free); - k_add_head(optioncontrol_free, oc_item); - K_WUNLOCK(optioncontrol_free); - } snprintf(reply, siz, "ERR.%s", reason); LOGERR("%s.%s.%s", cmd, id, reply); return strdup(reply); diff --git a/src/ckdb_dbio.c b/src/ckdb_dbio.c index 79ddfb85..471d5137 100644 --- a/src/ckdb_dbio.c +++ b/src/ckdb_dbio.c @@ -1841,7 +1841,7 @@ K_ITEM *optioncontrol_item_add(PGconn *conn, K_ITEM *oc_item, tv_t *cd, bool beg K_TREE_CTX ctx[1]; PGresult *res; K_ITEM *old_item, look; - OPTIONCONTROL *row; + OPTIONCONTROL *row, *optioncontrol; char *upd, *ins; bool ok = false; char *params[4 + HISTORYDATECOUNT]; @@ -1935,13 +1935,17 @@ nostart: free(params[n]); K_WLOCK(optioncontrol_free); - if (!ok) + if (!ok) { + // Cleanup item passed in + FREENULL(row->optionvalue); k_add_head(optioncontrol_free, oc_item); - else { - // Discard it + } else { + // Discard old if (old_item) { + DATA_OPTIONCONTROL(optioncontrol, old_item); optioncontrol_root = remove_from_ktree(optioncontrol_root, old_item, cmp_optioncontrol); + FREENULL(optioncontrol->optionvalue); k_add_head(optioncontrol_free, old_item); } optioncontrol_root = add_to_ktree(optioncontrol_root, oc_item, cmp_optioncontrol); @@ -1962,7 +1966,6 @@ K_ITEM *optioncontrol_add(PGconn *conn, char *optionname, char *optionvalue, { K_ITEM *item; OPTIONCONTROL *row; - bool ok = false; LOGDEBUG("%s(): add", __func__); @@ -1990,19 +1993,7 @@ K_ITEM *optioncontrol_add(PGconn *conn, char *optionname, char *optionvalue, HISTORYDATEINIT(row, cd, by, code, inet); HISTORYDATETRANSFER(trf_root, row); - ok = optioncontrol_item_add(conn, item, cd, begun); - - if (!ok) { - free(row->optionvalue); - K_WLOCK(optioncontrol_free); - k_add_head(optioncontrol_free, item); - K_WUNLOCK(optioncontrol_free); - } - - if (ok) - return item; - else - return NULL; + return optioncontrol_item_add(conn, item, cd, begun); } bool optioncontrol_fill(PGconn *conn) @@ -2084,8 +2075,10 @@ bool optioncontrol_fill(PGconn *conn) optioncontrol_root = add_to_ktree(optioncontrol_root, item, cmp_optioncontrol); k_add_head(optioncontrol_store, item); } - if (!ok) + if (!ok) { + FREENULL(row->optionvalue); k_add_head(optioncontrol_free, item); + } K_WUNLOCK(optioncontrol_free); PQclear(res); @@ -2148,10 +2141,8 @@ int64_t workinfo_add(PGconn *conn, char *workinfoidstr, char *poolinstance, K_WLOCK(workinfo_free); if (find_in_ktree(workinfo_root, item, cmp_workinfo, ctx)) { - free(row->transactiontree); - row->transactiontree = NULL; - free(row->merklehash); - row->merklehash = NULL; + FREENULL(row->transactiontree); + FREENULL(row->merklehash); workinfoid = row->workinfoid; k_add_head(workinfo_free, item); K_WUNLOCK(workinfo_free); @@ -2213,10 +2204,8 @@ unparam: K_WLOCK(workinfo_free); if (workinfoid == -1) { - free(row->transactiontree); - row->transactiontree = NULL; - free(row->merklehash); - row->merklehash = NULL; + FREENULL(row->transactiontree); + FREENULL(row->merklehash); k_add_head(workinfo_free, item); } else { if (row->transactiontree && *(row->transactiontree)) { @@ -3165,8 +3154,7 @@ bool sharesummary_fill(PGconn *conn) DATA_SHARESUMMARY(row, item); if (row->workername) { LIST_MEM_SUB(sharesummary_free, row->workername); - free(row->workername); - row->workername = NULL; + FREENULL(row->workername); } k_add_head(sharesummary_free, item); }