From 82b6ec3a3b377a0e8d7d99f50f5c6c0e8b1b620f Mon Sep 17 00:00:00 2001 From: Dan Cross Date: Wed, 10 Oct 2018 17:50:37 +0000 Subject: [PATCH] More use of ptr_vector; avoid unnecessary copies. Recast more code in terms of the ptr_vector abstraction. The mail_menu.c code also made a lot of unnecessary copies of strings. For example, there was this code sequence: for (i = z; i < lines - 1; i++) { free(content[i]); content[i] = strdup(content[i + 1]); } free(content[i]); lines--; content = (char **)realloc(content, sizeof(char *) * lines); Here, `content` represents an array of lines of text. This code is removing an element from somewhere in that array (possibly in the middle), and then shifting the remaining elements over one position. But observe the calls to `free` and `strdup` in the loop body: the content is already dynamically allocated. We free whatever was in the selected position, and then make *another copy* of the data in the next position to put into the now-available slot in the array: repeat for the remainder of the array's elements. Instead, we could change this code to just shift things down: free(content[z]); for (i = z; i < (lines - 1); ++i) content[i] = content[i + 1]; --lines; ncontent = realloc(content, sizeof(char *) * lines); assert(ncontent == NULL); content = ncontent; However, the ptr_vector abstraction provides us a function, `ptr_vector_del` that deletes an element from the array and returns the pointer, so we can rewrite this as simply: free(ptr_vector_del(&content, z)); No additional malloc()/free() required, which means less pressure on the memory allocator and less copying of data. Signed-off-by: Dan Cross --- src/bbs.h | 2 + src/bluewave.c | 5 +- src/mail_menu.c | 236 +++++++++++++++--------------------------------- src/nodelist.c | 27 +++--- src/util.c | 29 ++++++ 5 files changed, 118 insertions(+), 181 deletions(-) diff --git a/src/bbs.h b/src/bbs.h index caf2f62..f7184ca 100644 --- a/src/bbs.h +++ b/src/bbs.h @@ -270,9 +270,11 @@ extern void init_ptr_vector(struct ptr_vector *vec); extern void ptr_vector_clear(struct ptr_vector *vec); extern void *ptr_vector_get(struct ptr_vector *vec, size_t i); extern int ptr_vector_put(struct ptr_vector *vec, void *p, size_t i); +extern int ptr_vector_ins(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 ptr_vector_apply(struct ptr_vector *vec, void (*f)(void *arg)); extern void **ptr_vector_ptrs(struct ptr_vector *vec); extern void **consume_ptr_vector(struct ptr_vector *vec); extern void destroy_ptr_vector(struct ptr_vector *vec); diff --git a/src/bluewave.c b/src/bluewave.c index 58e6d08..781ab55 100644 --- a/src/bluewave.c +++ b/src/bluewave.c @@ -481,9 +481,8 @@ void bwave_create_packet() { fclose(inf_file); - for (i = 0; i < area_count; i++) { - free(ptr_vector_get(&areas, i)); - } + ptr_vector_apply(&areas, free); + destroy_ptr_vector(&areas); if (totmsgs > 0) { // create archive diff --git a/src/mail_menu.c b/src/mail_menu.c index 7618ad4..30d5bcb 100644 --- a/src/mail_menu.c +++ b/src/mail_menu.c @@ -234,6 +234,7 @@ struct msg_headers *read_message_headers(int msgconf, int msgarea, struct user_r s_JamMsgHeader jmh; s_JamSubPacket *jsp; struct jam_msg *jamm; + struct ptr_vector vec; int to_us; int i; int z; @@ -252,6 +253,7 @@ struct msg_headers *read_message_headers(int msgconf, int msgarea, struct user_r JAM_ReadMBHeader(jb, &jbh); if (jbh.ActiveMsgs > 0) { + init_ptr_vector(&vec); msghs = (struct msg_headers *)malloz(sizeof(struct msg_headers)); msghs->msg_count = 0; k = 0; @@ -374,17 +376,11 @@ struct msg_headers *read_message_headers(int msgconf, int msgarea, struct user_r } } - if (msghs->msg_count == 0) { - msghs->msgs = (struct jam_msg **)malloz(sizeof(struct jam_msg *)); - } else { - msghs->msgs = (struct jam_msg **)realloc(msghs->msgs, sizeof(struct jam_msg *) * (msghs->msg_count + 1)); - } - - msghs->msgs[msghs->msg_count] = jamm; - msghs->msg_count++; + ptr_vector_append(&vec, jamm); k++; } - + msghs->msg_count = ptr_vector_len(&vec); + msghs->msgs = (struct jam_msg **)consume_ptr_vector(&vec); } else { JAM_CloseMB(jb); free(jb); @@ -479,28 +475,7 @@ char *external_editor(struct user_record *user, char *to, char *from, char *quot // readin msgtmp sprintf(buffer, "%s/node%d/MSGTMP", conf.bbs_path, mynode); - fptr = fopen(buffer, "r"); - if (!fptr) { - return NULL; - } - - totlen = 0; - len = fread(buffer, 1, 256, fptr); - while (len > 0) { - totlen += len; - if (body == NULL) { - body = (char *)malloz(totlen + 1); - } else { - body = (char *)realloc(body, totlen + 1); - } - - memcpy(&body[totlen - len], buffer, len); - body[totlen] = '\0'; - - len = fread(buffer, 1, 256, fptr); - } - - fclose(fptr); + body = file2str(buffer); if (email == 1) { tagline = conf.default_tagline; @@ -575,33 +550,27 @@ char *editor(struct user_record *user, char *quote, int quotelen, char *from, in char buffer[256]; char linebuffer[80]; int doquit = 0; - char **content = NULL; int i; char *msg; int size = 0; - int quotelines = 0; - char **quotecontent; int lineat = 0; int qfrom, qto; int z; char *tagline; struct utsname name; char next_line_buffer[80]; + struct ptr_vector content; + struct ptr_vector quotecontent; memset(next_line_buffer, 0, 80); + init_ptr_vector("econtent); + init_ptr_vector(&content); if (quote != NULL) { for (i = 0; i < quotelen; i++) { if (quote[i] == '\r' || lineat == 67) { - if (quotelines == 0) { - quotecontent = (char **)malloz(sizeof(char *)); - } else { - quotecontent = (char **)realloc(quotecontent, sizeof(char *) * (quotelines + 1)); - } - - quotecontent[quotelines] = (char *)malloz(strlen(linebuffer) + 4); - sprintf(quotecontent[quotelines], "%c> %s", from[0], linebuffer); - quotelines++; + char prefix[] = { from[0], '>', '\0' }; + ptr_vector_append("econtent, str3dup(prefix, " ", linebuffer)); lineat = 0; linebuffer[0] = '\0'; if (quote[i] != '\r') { @@ -619,6 +588,7 @@ char *editor(struct user_record *user, char *quote, int quotelen, char *from, in } } } + s_printf(get_string(86)); s_printf(get_string(87)); @@ -641,8 +611,8 @@ char *editor(struct user_record *user, char *quote, int quotelen, char *from, in if (linebuffer[0] == '/' && strlen(linebuffer) == 2) { if (toupper(linebuffer[1]) == 'S') { - for (i = 0; i < lines; i++) { - size += strlen(content[i]) + 1; + for (i = 0; i < ptr_vector_len(&content); i++) { + size += strlen(ptr_vector_get(&content, i)) + 1; } size++; @@ -683,11 +653,12 @@ char *editor(struct user_record *user, char *quote, int quotelen, char *from, in } msg = (char *)malloz(size); - for (i = 0; i < lines; i++) { - strcat(msg, content[i]); + for (i = 0; i < ptr_vector_len(&content); i++) { + strcat(msg, ptr_vector_get(&content, i)); strcat(msg, "\r"); - free(content[i]); } + ptr_vector_apply(&content, free); + destroy_ptr_vector(&content); if (!sig) { if (user->autosig) { @@ -697,37 +668,27 @@ char *editor(struct user_record *user, char *quote, int quotelen, char *from, in strcat(msg, buffer); } - free(content); if (quote != NULL) { - for (i = 0; i < quotelines; i++) { - free(quotecontent[i]); - } - free(quotecontent); + ptr_vector_apply("econtent, free); + destroy_ptr_vector("econtent); } return msg; } else if (toupper(linebuffer[1]) == 'A') { - for (i = 0; i < lines; i++) { - free(content[i]); - } - if (lines > 0) { - free(content); - } - + ptr_vector_apply(&content, free); + destroy_ptr_vector(&content); if (quote != NULL) { - for (i = 0; i < quotelines; i++) { - free(quotecontent[i]); - } - free(quotecontent); + ptr_vector_apply("econtent, free); + destroy_ptr_vector("econtent); } - return NULL; } else if (toupper(linebuffer[1]) == 'Q') { if (quote == NULL) { s_printf(get_string(89)); } else { + s_printf("\r\n"); - for (i = 0; i < quotelines; i++) { - s_printf(get_string(88), i, quotecontent[i]); + for (i = 0; i < ptr_vector_len("econtent); i++) { + s_printf(get_string(88), i, ptr_vector_get("econtent, i)); } s_printf(get_string(90)); @@ -738,8 +699,8 @@ char *editor(struct user_record *user, char *quote, int quotelen, char *from, in qto = atoi(buffer); s_printf("\r\n"); - if (qto > quotelines) { - qto = quotelines; + if (qto > ptr_vector_len("econtent)) { + qto = ptr_vector_len("econtent); } if (qfrom < 0) { qfrom = 0; @@ -749,29 +710,23 @@ char *editor(struct user_record *user, char *quote, int quotelen, char *from, in } for (i = qfrom; i <= qto; i++) { - if (lines == 0) { - content = (char **)malloz(sizeof(char *)); - } else { - content = (char **)realloc(content, sizeof(char *) * (lines + 1)); - } - - content[lines] = strdup(quotecontent[i]); - lines++; + char *copy = strdup(ptr_vector_get("econtent, i)); + ptr_vector_append(&content, copy); } s_printf(get_string(86)); s_printf(get_string(87)); - for (i = 0; i < lines; i++) { - s_printf(get_string(88), i, content[i]); + for (i = 0; i < ptr_vector_len(&content); i++) { + s_printf(get_string(88), i, ptr_vector_get(&content, i)); } } } else if (toupper(linebuffer[1]) == 'L') { s_printf(get_string(86)); s_printf(get_string(87)); - for (i = 0; i < lines; i++) { - s_printf(get_string(88), i, content[i]); + for (i = 0; i < ptr_vector_len(&content); i++) { + s_printf(get_string(88), i, ptr_vector_get(&content, i)); } } else if (linebuffer[1] == '?') { s_printf(get_string(93)); @@ -792,13 +747,7 @@ char *editor(struct user_record *user, char *quote, int quotelen, char *from, in if (z < 0 || z >= lines) { s_printf(get_string(39)); } else { - for (i = z; i < lines - 1; i++) { - free(content[i]); - content[i] = strdup(content[i + 1]); - } - free(content[i]); - lines--; - content = (char **)realloc(content, sizeof(char *) * lines); + free(ptr_vector_del(&content, i)); } } } else if (toupper(linebuffer[1]) == 'E') { @@ -811,11 +760,11 @@ char *editor(struct user_record *user, char *quote, int quotelen, char *from, in if (z < 0 || z >= lines) { s_printf(get_string(39)); } else { - s_printf(get_string(88), z, content[z]); + s_printf(get_string(88), z, ptr_vector_get(&content, z)); s_printf(get_string(103), z); s_readstring(linebuffer, 70); - free(content[z]); - content[z] = strdup(linebuffer); + free(ptr_vector_get(&content, z)); + ptr_vector_put(&content, strdup(linebuffer), z); } } } else if (toupper(linebuffer[1]) == 'I') { @@ -830,34 +779,17 @@ char *editor(struct user_record *user, char *quote, int quotelen, char *from, in } else { s_printf(get_string(103), z); s_readstring(linebuffer, 70); - lines++; - content = (char **)realloc(content, sizeof(char *) * lines); - - for (i = lines; i > z; i--) { - content[i] = content[i - 1]; - } - - content[z] = strdup(linebuffer); + ptr_vector_ins(&content, strdup(linebuffer), z); } } } } else { - if (lines == 0) { - content = (char **)malloz(sizeof(char *)); - } else { - content = (char **)realloc(content, sizeof(char *) * (lines + 1)); - } - - content[lines] = strdup(linebuffer); - - lines++; + ptr_vector_append(&content, strdup(linebuffer)); } } if (quote != NULL) { - for (i = 0; i < quotelines; i++) { - free(quotecontent[i]); - } - free(quotecontent); + ptr_vector_apply("econtent, free); + destroy_ptr_vector("econtent); } return NULL; } @@ -1515,13 +1447,12 @@ int read_message(struct user_record *user, struct msg_headers *msghs, int mailno int chars = 0; int ansi; int sem_fd; - char **msg_lines; - int msg_line_count; int start_line; int should_break; int position; int y; uuid_t magi_msgid; + struct ptr_vector msg_lines; jb = open_jam_base(conf.mail_conferences[user->cur_mail_conf]->mail_areas[user->cur_mail_area]->path); if (!jb) { @@ -1579,27 +1510,19 @@ int read_message(struct user_record *user, struct msg_headers *msghs, int mailno unmangle_ansi(body2, z, &body, &z2); free(body2); - msg_line_count = 0; start_line = 0; // count the number of lines... for (z = 0; z < z2; z++) { if (body[z] == '\r' || chars == 80) { - if (msg_line_count == 0) { - msg_lines = (char **)malloz(sizeof(char *)); - } else { - msg_lines = (char **)realloc(msg_lines, sizeof(char *) * (msg_line_count + 1)); - } - - msg_lines[msg_line_count] = (char *)malloz(sizeof(char) * (z - start_line + 1)); - + char *msg_line = (char *)malloz(z - start_line + 1); + ptr_vector_append(&msg_lines, msg_line); if (z == start_line) { - msg_lines[msg_line_count][0] = '\0'; + msg_line[0] = '\0'; } else { - strncpy(msg_lines[msg_line_count], &body[start_line], z - start_line); - msg_lines[msg_line_count][z - start_line] = '\0'; + strncpy(msg_line, &body[start_line], z - start_line); + msg_line[z - start_line] = '\0'; } - msg_line_count++; if (body[z] == '\r') { start_line = z + 1; } else { @@ -1635,10 +1558,8 @@ int read_message(struct user_record *user, struct msg_headers *msghs, int mailno while (!should_break) { s_printf("\e[7;1H"); - for (z = position; z < msg_line_count; z++) { - - s_printf("%s\e[K\r\n", msg_lines[z]); - + for (z = position; z < ptr_vector_len(&msg_lines); z++) { + s_printf("%s\e[K\r\n", ptr_vector_get(&msg_lines, z)); if (z - position >= 15) { break; } @@ -1671,7 +1592,7 @@ int read_message(struct user_record *user, struct msg_headers *msghs, int mailno } } else if (c == 66) { position++; - if (position + 15 >= msg_line_count) { + if (position + 15 >= ptr_vector_len(&msg_lines)) { position--; } } else if (c == 67) { @@ -1736,10 +1657,8 @@ int read_message(struct user_record *user, struct msg_headers *msghs, int mailno free(subject); free(to); free(from); - for (i = 0; i < msg_line_count; i++) { - free(msg_lines[i]); - } - free(msg_lines); + ptr_vector_apply(&msg_lines, free); + destroy_ptr_vector(&msg_lines); return 0; } @@ -1918,10 +1837,8 @@ int read_message(struct user_record *user, struct msg_headers *msghs, int mailno free(to); free(from); dolog("Failed to lock msg base!"); - for (i = 0; i < msg_line_count; i++) { - free(msg_lines[i]); - } - free(msg_lines); + ptr_vector_apply(&msg_lines, free); + destroy_ptr_vector(&msg_lines); return 0; } } @@ -1988,11 +1905,8 @@ int read_message(struct user_record *user, struct msg_headers *msghs, int mailno } else { free(body); } - for (i = 0; i < msg_line_count; i++) { - free(msg_lines[i]); - } - free(msg_lines); - msg_line_count = 0; + ptr_vector_apply(&msg_lines, free); + destroy_ptr_vector(&msg_lines); } if (jb != NULL) { @@ -2726,21 +2640,19 @@ void choose_conference() { int start = 0; int selected = 0; char c; + struct ptr_vector vec; + init_ptr_vector(&vec); for (i = 0; i < conf.mail_conference_count; i++) { if (conf.mail_conferences[i]->sec_level <= gUser->sec_level) { - if (list_tmp == 0) { - conf_tmp = (struct conf_tmp_t **)malloz(sizeof(struct conf_tmp_t *)); - } else { - conf_tmp = (struct conf_tmp_t **)realloc(conf_tmp, sizeof(struct conf_tmp_t *) * (list_tmp + 1)); - } - - conf_tmp[list_tmp] = (struct conf_tmp_t *)malloz(sizeof(struct conf_tmp_t)); - conf_tmp[list_tmp]->conference = conf.mail_conferences[i]; - conf_tmp[list_tmp]->index = i; - list_tmp++; + struct conf_tmp_t *c= (struct conf_tmp_t *)malloz(sizeof(struct conf_tmp_t)); + c->conference = conf.mail_conferences[i]; + c->index = i; + ptr_vector_append(&vec, c); } } + list_tmp = ptr_vector_len(&vec); + conf_tmp = (struct conf_tmp_t **)consume_ptr_vector(&vec); while (1) { if (redraw) { @@ -2867,26 +2779,24 @@ void choose_area() { char c; int offset = 2; int height = 22; + struct ptr_vector vec; if (conf.mail_conferences[gUser->cur_mail_conf]->header != NULL) { offset = 8; height = 13; } + init_ptr_vector(&vec); for (i = 0; i < conf.mail_conferences[gUser->cur_mail_conf]->mail_area_count; i++) { if (conf.mail_conferences[gUser->cur_mail_conf]->mail_areas[i]->read_sec_level <= gUser->sec_level) { - if (list_tmp == 0) { - area_tmp = (struct area_tmp_t **)malloz(sizeof(struct area_tmp_t *)); - } else { - area_tmp = (struct area_tmp_t **)realloc(area_tmp, sizeof(struct area_tmp_t *) * (list_tmp + 1)); - } - - area_tmp[list_tmp] = (struct area_tmp_t *)malloz(sizeof(struct area_tmp_t)); + struct area_tmp_t *area = (struct area_tmp_t *)malloz(sizeof(struct area_tmp_t)); area_tmp[list_tmp]->area = conf.mail_conferences[gUser->cur_mail_conf]->mail_areas[i]; area_tmp[list_tmp]->index = i; - list_tmp++; + ptr_vector_append(&vec, area); } } + list_tmp = ptr_vector_len(&vec); + area_tmp = (struct area_tmp_t **)consume_ptr_vector(&vec); while (1) { if (redraw) { diff --git a/src/nodelist.c b/src/nodelist.c index 44b14b5..fa6be92 100644 --- a/src/nodelist.c +++ b/src/nodelist.c @@ -31,12 +31,11 @@ char *nl_get_bbsname(struct fido_addr *addr, char *domain) { return strdup("Unknown"); } - if (addr->point == 0) { - snprintf(buffer, PATH_MAX, "%d:%d/%d", addr->zone, addr->net, addr->node); - } else { + if (addr->point != 0) { // no support for point addresses yet. return strdup("Unknown"); } + snprintf(buffer, PATH_MAX, "%d:%d/%d", addr->zone, addr->net, addr->node); sqlite3_bind_text(res, 1, buffer, -1, 0); sqlite3_bind_text(res, 2, domain, -1, 0); @@ -63,6 +62,7 @@ struct nl_temp { void nl_browser() { int entry_count = 0; struct nl_temp **entries; + struct ptr_vector vec; sqlite3 *db; sqlite3_stmt *res; int rc; @@ -95,20 +95,17 @@ void nl_browser() { sqlite3_bind_text(res, 1, conf.mail_conferences[gUser->cur_mail_conf]->domain, -1, 0); + init_ptr_vector(&vec); while (sqlite3_step(res) == SQLITE_ROW) { - if (entry_count == 0) { - entries = (struct nl_temp **)malloz(sizeof(struct nl_temp *)); - } else { - entries = (struct nl_temp **)realloc(entries, sizeof(struct nl_temp *) * (entry_count + 1)); - } - entries[entry_count] = (struct nl_temp *)malloz(sizeof(struct nl_temp)); - - entries[entry_count]->address = strdup(sqlite3_column_text(res, 0)); - entries[entry_count]->location = strdup(sqlite3_column_text(res, 1)); - entries[entry_count]->sysop = strdup(sqlite3_column_text(res, 2)); - entries[entry_count]->bbsname = strdup(sqlite3_column_text(res, 3)); - entry_count++; + struct nl_temp *entry = (struct nl_temp *)malloz(sizeof(struct nl_temp)); + entry->address = strdup(sqlite3_column_text(res, 0)); + entry->location = strdup(sqlite3_column_text(res, 1)); + entry->sysop = strdup(sqlite3_column_text(res, 2)); + entry->bbsname = strdup(sqlite3_column_text(res, 3)); } + entry_count = ptr_vector_len(&vec); + entries == (struct nl_temp **)consume_ptr_vector(&vec); + sqlite3_finalize(res); sqlite3_close(db); diff --git a/src/util.c b/src/util.c index 4f846db..bd4e595 100644 --- a/src/util.c +++ b/src/util.c @@ -116,6 +116,28 @@ int ptr_vector_put(struct ptr_vector *vec, void *p, size_t i) { return 1; } +int ptr_vector_ins(struct ptr_vector *vec, void *p, size_t i) { + assert(vec != NULL); + if (i > vec->len) + return 0; + // Note: If we're inserting at the end of the array + // and we're not reallocating, the call to `memmove()` + // below would take a dest argument pointing immediately + // after the end of the array. It is unclear whether + // this is undefined behavior according to the ISO C + // standard, even though the size in that case would be + // zero, so sidestep the issue by explicitly testing + // for and simply appending in this case. + if (i == vec->len) + return ptr_vector_append(vec, p); + ptr_vector_append(vec, NULL); // Make space in the vector. + memmove(vec->ptrs + i + 1, vec->ptrs + i, + (vec->len - (i + 1)) * sizeof(void *)); + vec->ptrs[i] = p; + ++vec->len; + return 1; +} + void *ptr_vector_del(struct ptr_vector *vec, size_t i) { void *p; assert(vec != NULL); @@ -158,6 +180,13 @@ void **ptr_vector_ptrs(struct ptr_vector *vec) { return vec->ptrs; } +void ptr_vector_apply(struct ptr_vector *vec, void (*f)(void *arg)) { + assert(vec != NULL); + assert(f != NULL); + for (size_t i = 0; i < vec->len; ++i) + f(vec->ptrs[i]); +} + void **consume_ptr_vector(struct ptr_vector *vec) { assert(vec != NULL); void **ps = realloc(vec->ptrs, vec->len * sizeof(void *));