From 540e359080e99b5d410dda5d02d8907eb7faf552 Mon Sep 17 00:00:00 2001 From: Dan Cross Date: Tue, 9 Oct 2018 15:55:15 +0000 Subject: [PATCH] Cleanups and pointer vectors. A repeated pattern in Magicka is to append to dynamically sized arrays via malloc()/realloc(). Introduce the notion of a "pointer vector": that is, a growable vector of pointers, that can be reused to implement that logic more safely and efficiently (this implementation uses power-of-two growing). Many malloc()/realloc() calls were not checked; these assert() that the return value from realloc() is not NULL. Add a method to consume the pointer vector: that is, realloc() it to the current length and return the underlying pointers. Make the `fmt` argument to dolog() const. Include in bluewave.c to squash a warning. Signed-off-by: Dan Cross --- src/bbs.c | 2 +- src/bbs.h | 3 +- src/bbs_list.c | 78 ++++++++++++++++++++-------------------------- src/blog.c | 25 +++++++-------- src/bluewave.c | 84 ++++++++++++++++++++++++-------------------------- src/doors.c | 21 ++++++------- src/util.c | 10 ++++++ 7 files changed, 109 insertions(+), 114 deletions(-) diff --git a/src/bbs.c b/src/bbs.c index af071a4..03fff29 100644 --- a/src/bbs.c +++ b/src/bbs.c @@ -88,7 +88,7 @@ void dolog_www(char *ipaddr, char *fmt, ...) { fclose(logfptr); } -void dolog(char *fmt, ...) { +void dolog(const char *fmt, ...) { char buffer[PATH_MAX]; struct tm time_now; time_t timen; diff --git a/src/bbs.h b/src/bbs.h index 6f26a44..e728dae 100644 --- a/src/bbs.h +++ b/src/bbs.h @@ -273,6 +273,7 @@ extern int ptr_vector_put(struct ptr_vector *vec, void *p, size_t i); extern void *ptr_vector_del(struct ptr_vector *vec, size_t i); extern int ptr_vector_append(struct ptr_vector *vec, void *p); extern size_t ptr_vector_len(struct ptr_vector *vec); +extern void **consume_ptr_vector(struct ptr_vector *vec); extern void destroy_ptr_vector(struct ptr_vector *vec); extern char *str_replace(const char *orig, const char *rep, const char *with); @@ -281,7 +282,7 @@ extern int recursive_delete(const char *dir); extern void automessage_write(); extern void automessage_display(); extern void automessage(); -extern void dolog(char *fmt, ...); +extern void dolog(const char *fmt, ...); extern void dolog_www(char *ipaddr, char *fmt, ...); extern void runbbs_ssh(char *ipaddress); extern void runbbs(int sock, char *ipaddress); diff --git a/src/bbs_list.c b/src/bbs_list.c index 127ba31..ad86467 100644 --- a/src/bbs_list.c +++ b/src/bbs_list.c @@ -189,10 +189,11 @@ void bbs_list() { sqlite3_stmt *res; int rc; char *sql = "SELECT id,bbsname,sysop,telnet FROM bbslist"; - struct bbs_list_entry_t **entries; + struct ptr_vector entries; int entrycount; struct bbs_list_entry_t *newentry; + init_ptr_vector(&entries); while (1) { entrycount = 0; snprintf(buffer, PATH_MAX, "%s/bbslist.sq3", conf.bbs_path); @@ -211,22 +212,17 @@ void bbs_list() { } else { while (sqlite3_step(res) == SQLITE_ROW) { - if (entrycount == 0) { - entries = (struct bbs_list_entry_t **)malloz(sizeof(struct bbs_list_entry_t *)); - } else { - entries = (struct bbs_list_entry_t **)realloc(entries, sizeof(struct bbs_list_entry_t *) * (entrycount + 1)); - } - entries[entrycount] = (struct bbs_list_entry_t *)malloz(sizeof(struct bbs_list_entry_t)); - - entries[entrycount]->id = sqlite3_column_int(res, 0); - entries[entrycount]->bbsname = strdup(sqlite3_column_text(res, 1)); - entries[entrycount]->sysopname = strdup(sqlite3_column_text(res, 2)); - entries[entrycount]->telnet = strdup(sqlite3_column_text(res, 3)); - entrycount++; + struct bbs_list_entry_t *entry = malloz(sizeof(struct bbs_list_entry_t)); + entry->id = sqlite3_column_int(res, 0); + entry->bbsname = strdup(sqlite3_column_text(res, 1)); + entry->sysopname = strdup(sqlite3_column_text(res, 2)); + entry->telnet = strdup(sqlite3_column_text(res, 3)); + ptr_vector_append(&entries, entry); } sqlite3_finalize(res); sqlite3_close(db); } + entrycount = ptr_vector_len(&entries); if (entrycount > 0) { while (1) { @@ -235,11 +231,9 @@ void bbs_list() { s_printf(get_string(270)); s_printf(get_string(271)); for (i = start; i < start + 22 && i < entrycount; i++) { - if (i == selected) { - s_printf(get_string(269), i - start + 2, i, entries[i]->bbsname, entries[i]->sysopname, entries[i]->telnet); - } else { - s_printf(get_string(268), i - start + 2, i, entries[i]->bbsname, entries[i]->sysopname, entries[i]->telnet); - } + struct bbs_list_entry_t *entry = ptr_vector_get(&entries, i); + int strn = (i == selected) ? 269 : 268; + s_printf(get_string(strn), i - start + 2, i, entry->bbsname, entry->sysopname, entry->telnet); } s_printf("\e[%d;5H", selected - start + 2); redraw = 0; @@ -247,42 +241,34 @@ void bbs_list() { c = s_getchar(); if (tolower(c) == 'q') { for (i = 0; i < entrycount; i++) { - free(entries[i]->bbsname); - free(entries[i]->sysopname); - free(entries[i]->telnet); - free(entries[i]); + struct bbs_list_entry_t *entry = ptr_vector_get(&entries, i); + free(entry->bbsname); + free(entry->sysopname); + free(entry->telnet); + free(entry); } - free(entries); + destroy_ptr_vector(&entries); return; } else if (tolower(c) == 'a') { newentry = (struct bbs_list_entry_t *)malloz(sizeof(struct bbs_list_entry_t)); if (add_bbs(newentry)) { - entries = (struct bbs_list_entry_t **)realloc(entries, sizeof(struct bbs_list_entry_t *) * (entrycount + 1)); - entries[entrycount] = newentry; + ptr_vector_append(&entries, newentry); entrycount++; - redraw = 1; } else { free(newentry); - redraw = 1; } + redraw = 1; } else if (tolower(c) == 'd') { - if (delete_bbs(entries[selected]->id)) { - free(entries[selected]->bbsname); - free(entries[selected]->sysopname); - free(entries[selected]->telnet); - free(entries[selected]); - - for (i = selected; i < entrycount - 1; i++) { - entries[selected] = entries[selected + 1]; - } - + struct bbs_list_entry_t *entry = ptr_vector_del(&entries, selected); + if (delete_bbs(entry->id)) { + free(entry->bbsname); + free(entry->sysopname); + free(entry->telnet); + free(entry); entrycount--; if (entrycount == 0) { - free(entries); return; } - entries = (struct bbs_list_entry_t **)realloc(entries, sizeof(struct bbs_list_entry_t *) * entrycount); - if (selected >= entrycount) { selected = entrycount - 1; } @@ -306,8 +292,10 @@ void bbs_list() { selected = entrycount - 1; } else { if (!redraw) { - s_printf(get_string(268), selected - start + 1, selected - 1, entries[selected - 1]->bbsname, entries[selected - 1]->sysopname, entries[selected - 1]->telnet); - s_printf(get_string(269), selected - start + 2, selected, entries[selected]->bbsname, entries[selected]->sysopname, entries[selected]->telnet); + struct bbs_list_entry_t *before = ptr_vector_get(&entries, selected - 1); + struct bbs_list_entry_t *entry = ptr_vector_get(&entries, selected); + s_printf(get_string(268), selected - start + 1, selected - 1, before->bbsname, before->sysopname, before->telnet); + s_printf(get_string(269), selected - start + 2, selected, entry->bbsname, entry->sysopname, entry->telnet); s_printf("\e[%d;4H", selected - start + 2); } } @@ -325,8 +313,10 @@ void bbs_list() { selected = 0; } else { if (!redraw) { - s_printf(get_string(269), selected - start + 2, selected, entries[selected]->bbsname, entries[selected]->sysopname, entries[selected]->telnet); - s_printf(get_string(268), selected - start + 3, selected + 1, entries[selected + 1]->bbsname, entries[selected + 1]->sysopname, entries[selected + 1]->telnet); + struct bbs_list_entry_t *entry = ptr_vector_get(&entries, selected); + struct bbs_list_entry_t *after = ptr_vector_get(&entries, selected + 1); + s_printf(get_string(269), selected - start + 2, selected, entry->bbsname, entry->sysopname, entry->telnet); + s_printf(get_string(268), selected - start + 3, selected + 1, after->bbsname, after->sysopname, after->telnet); s_printf("\e[%d;4H", selected - start + 2); } } diff --git a/src/blog.c b/src/blog.c index 6bc1abf..3bb8831 100644 --- a/src/blog.c +++ b/src/blog.c @@ -8,7 +8,7 @@ extern struct user_record *gUser; int blog_load(struct blog_entry_t ***entries) { int blog_entry_count = 0; - struct blog_entry_t **blog_entries; + struct ptr_vector blog_entries; char *sql = "SELECT author, title, body, date FROM blog ORDER BY date DESC"; char buffer[PATH_MAX]; @@ -34,25 +34,22 @@ int blog_load(struct blog_entry_t ***entries) { *entries = NULL; return 0; } + init_ptr_vector(&blog_entries); while (sqlite3_step(res) == SQLITE_ROW) { - if (blog_entry_count == 0) { - blog_entries = (struct blog_entry_t **)malloz(sizeof(struct blog_entry_t *)); - } else { - blog_entries = (struct blog_entry_t **)realloc(blog_entries, sizeof(struct blog_entry_t *) * (blog_entry_count + 1)); - } - blog_entries[blog_entry_count] = (struct blog_entry_t *)malloz(sizeof(struct blog_entry_t)); - - blog_entries[blog_entry_count]->author = strdup(sqlite3_column_text(res, 0)); - blog_entries[blog_entry_count]->subject = strdup(sqlite3_column_text(res, 1)); - blog_entries[blog_entry_count]->body = strdup(sqlite3_column_text(res, 2)); - blog_entries[blog_entry_count]->date = sqlite3_column_int(res, 3); - blog_entry_count++; + struct blog_entry_t *entry = (struct blog_entry_t *)malloz(sizeof(struct blog_entry_t)); + entry->author = strdup(sqlite3_column_text(res, 0)); + entry->subject = strdup(sqlite3_column_text(res, 1)); + entry->body = strdup(sqlite3_column_text(res, 2)); + entry->date = sqlite3_column_int(res, 3); + ptr_vector_append(&blog_entries, entry); } sqlite3_finalize(res); sqlite3_close(db); - *entries = blog_entries; + blog_entry_count = ptr_vector_len(&blog_entries); + *entries = consume_ptr_vector(&blog_entries); + return blog_entry_count; } diff --git a/src/bluewave.c b/src/bluewave.c index 774e644..2f6a1a8 100644 --- a/src/bluewave.c +++ b/src/bluewave.c @@ -5,6 +5,7 @@ #include #include #include +#include #include "bluewave.h" #include "jamlib/jam.h" #include "bbs.h" @@ -291,7 +292,8 @@ void bwave_create_packet() { char buffer[1024]; char archive[1024]; INF_HEADER hdr; - INF_AREA_INFO **areas = NULL; + struct ptr_vector areas; + INF_AREA_INFO *area = NULL; int i; int j; int area_count; @@ -315,11 +317,13 @@ void bwave_create_packet() { int tot_areas = 0; int totmsgs = 0; int ret; - char **args; - int arg_count; + char **args, *arg; + struct ptr_vector args_vec; char *cmd; pid_t pid; + init_ptr_vector(&areas); + for (i = 0; i < conf.mail_conference_count; i++) { for (j = 0; j < conf.mail_conferences[i]->mail_area_count; j++) { if (msgbase_is_subscribed(i, j)) { @@ -386,22 +390,22 @@ void bwave_create_packet() { totmsgs = bwave_scan_email(area_count + 1, totmsgs, fti_file, mix_file, dat_file, &last_ptr); s_printf(get_string(195), "Private Email", "Private Email", totmsgs); - areas = (INF_AREA_INFO **)malloz(sizeof(INF_AREA_INFO *)); flags = 0; - areas[area_count] = (INF_AREA_INFO *)malloz(sizeof(INF_AREA_INFO)); + area = (INF_AREA_INFO *)malloz(sizeof(INF_AREA_INFO)); - snprintf(areas[area_count]->areanum, 6, "%d", area_count + 1); + snprintf(area->areanum, 6, "%d", area_count + 1); - memcpy(areas[area_count]->echotag, "PRIVATE_EMAIL", 13); + memcpy(area->echotag, "PRIVATE_EMAIL", 13); - strncpy(areas[area_count]->title, "Private Email", 49); + strncpy(area->title, "Private Email", 49); flags |= INF_POST; flags |= INF_NO_PUBLIC; flags |= INF_SCANNING; - areas[area_count]->area_flags = converts(flags); - areas[area_count]->network_type = INF_NET_FIDONET; + area->area_flags = converts(flags); + area->network_type = INF_NET_FIDONET; + ptr_vector_append(&areas, area); area_count++; @@ -417,16 +421,14 @@ void bwave_create_packet() { // continue; //} - areas = (INF_AREA_INFO **)realloc(areas, sizeof(INF_AREA_INFO *) * (area_count + 1)); - flags = 0; - areas[area_count] = (INF_AREA_INFO *)malloz(sizeof(INF_AREA_INFO)); + area = (INF_AREA_INFO *)malloz(sizeof(INF_AREA_INFO)); - snprintf(areas[area_count]->areanum, 6, "%d", area_count + 1); + snprintf(area->areanum, 6, "%d", area_count + 1); - memcpy(areas[area_count]->echotag, conf.mail_conferences[i]->mail_areas[j]->qwkname, strlen(conf.mail_conferences[i]->mail_areas[j]->qwkname)); + memcpy(area->echotag, conf.mail_conferences[i]->mail_areas[j]->qwkname, strlen(conf.mail_conferences[i]->mail_areas[j]->qwkname)); - strncpy(areas[area_count]->title, conf.mail_conferences[i]->mail_areas[j]->name, 49); + strncpy(area->title, conf.mail_conferences[i]->mail_areas[j]->name, 49); if (conf.mail_conferences[i]->mail_areas[j]->write_sec_level <= gUser->sec_level) { flags |= INF_POST; @@ -449,9 +451,10 @@ void bwave_create_packet() { flags |= INF_SCANNING; - areas[area_count]->area_flags = converts(flags); - areas[area_count]->network_type = INF_NET_FIDONET; + area->area_flags = converts(flags); + area->network_type = INF_NET_FIDONET; + ptr_vector_append(&areas, area); area_count++; if (totmsgs == conf.bwave_max_msgs) { break; @@ -474,16 +477,13 @@ void bwave_create_packet() { fwrite(&hdr, sizeof(INF_HEADER), 1, inf_file); for (i = 0; i < area_count; i++) { - fwrite(areas[i], sizeof(INF_AREA_INFO), 1, inf_file); + fwrite(ptr_vector_get(&areas, i), sizeof(INF_AREA_INFO), 1, inf_file); } fclose(inf_file); for (i = 0; i < area_count; i++) { - free(areas[i]); - } - if (areas != NULL) { - free(areas); + free(ptr_vector_get(&areas, i)); } if (totmsgs > 0) { @@ -530,15 +530,15 @@ void bwave_create_packet() { dup2(bbs_stdin, STDIN_FILENO); } - args = (char **)malloz(sizeof(char *)); - arg_count = 0; - args[arg_count] = strtok(buffer, " "); - while (args[arg_count] != NULL) { - arg_count++; - args = (char **)realloc(args, sizeof(char *) * (arg_count + 1)); - args[arg_count] = strtok(NULL, " "); + init_ptr_vector(&args_vec); + arg = strtok(buffer, " "); + ptr_vector_append(&args_vec, arg); + while (arg != NULL) { + arg = strtok(NULL, " "); + ptr_vector_append(&args_vec, arg); } - cmd = strdup(args[0]); + args = (char **)consume_ptr_vector(&args_vec); + cmd = args[0]; pid = fork(); if (pid == 0) { execvp(cmd, args); @@ -548,7 +548,6 @@ void bwave_create_packet() { } else { ret = -1; } - free(cmd); free(args); if (sshBBS) { @@ -813,8 +812,8 @@ void bwave_upload_reply() { "seen INTEGER);"; char *isql = "INSERT INTO email (sender, recipient, subject, body, date, seen) VALUES(?, ?, ?, ?, ?, 0)"; char *err_msg = 0; - char **args; - int arg_count; + char **args, *arg; + struct ptr_vector args_vec; char *cmd; pid_t pid; int ret; @@ -867,15 +866,15 @@ void bwave_upload_reply() { dup2(bbs_stderr, STDERR_FILENO); dup2(bbs_stdin, STDIN_FILENO); } - args = (char **)malloz(sizeof(char *)); - arg_count = 0; - args[arg_count] = strtok(buffer, " "); - while (args[arg_count] != NULL) { - arg_count++; - args = (char **)realloc(args, sizeof(char *) * (arg_count + 1)); - args[arg_count] = strtok(NULL, " "); + init_ptr_vector(&args_vec); + arg = strtok(buffer, " "); + ptr_vector_append(&args_vec, arg); + while (arg != NULL) { + arg = strtok(NULL, " "); + ptr_vector_append(&args_vec, arg); } - cmd = strdup(args[0]); + args = (char **)consume_ptr_vector(&args_vec); + cmd = args[0]; pid = fork(); if (pid == 0) { execvp(cmd, args); @@ -885,7 +884,6 @@ void bwave_upload_reply() { } else { ret = -1; } - free(cmd); free(args); if (sshBBS) { diff --git a/src/doors.c b/src/doors.c index 274060c..ce8bb90 100644 --- a/src/doors.c +++ b/src/doors.c @@ -250,8 +250,8 @@ void runexternal(struct user_record *user, char *cmd, int stdio, char *argv[], c size_t ouc; size_t inc; size_t sz; - char **args; - int arg_count; + char **args, *arg; + struct ptr_vector args_vec; pid_t pid; int iac; char iac_binary_will[] = {IAC, IAC_WILL, IAC_TRANSMIT_BINARY, '\0'}; @@ -523,15 +523,15 @@ void runexternal(struct user_record *user, char *cmd, int stdio, char *argv[], c if (cwd != NULL) { chdir(cwd); } - args = (char **)malloz(sizeof(char *)); - arg_count = 0; - args[arg_count] = strtok(buffer, " "); - while (args[arg_count] != NULL) { - arg_count++; - args = (char **)realloc(args, sizeof(char *) * (arg_count + 1)); - args[arg_count] = strtok(NULL, " "); + init_ptr_vector(&args_vec); + arg = strtok(buffer, " "); + ptr_vector_append(&args_vec, arg); + while (arg != NULL) { + arg = strtok(NULL, " "); + ptr_vector_append(&args_vec, arg); } - cmd = strdup(args[0]); + args = (char **)consume_ptr_vector(&args_vec); + cmd = args[0]; pid = fork(); if (pid == 0) { execvp(cmd, args); @@ -541,7 +541,6 @@ void runexternal(struct user_record *user, char *cmd, int stdio, char *argv[], c } else { ret = -1; } - free(cmd); free(args); if (cwd != NULL) { chdir(conf.bbs_path); diff --git a/src/util.c b/src/util.c index f23e3d9..ec7b85a 100644 --- a/src/util.c +++ b/src/util.c @@ -153,6 +153,16 @@ size_t ptr_vector_len(struct ptr_vector *vec) { return vec->len; } +void **consume_ptr_vector(struct ptr_vector *vec) { + assert(vec != NULL); + void **ps = realloc(vec->ptrs, vec->len * sizeof(void *)); + assert(ps != NULL); + vec->ptrs = NULL; + vec->len = 0; + vec->capacity = 0; + return ps; +} + void destroy_ptr_vector(struct ptr_vector *vec) { assert(vec != NULL); free(vec->ptrs);