From 1464a5879a5f18c1de507669dbe8130ebde9d5ab Mon Sep 17 00:00:00 2001 From: Art Cancro Date: Tue, 15 Aug 2023 18:14:24 -0900 Subject: [PATCH] DRY fetch of msglists. A lot of the calls to cdb_fetch(CDB_MSGLISTS...) follow the same design pattern and many of them are in need of rework to handle how they free their memory. So I created a new function CtdlGetMsgList() to do it in a consistent way. Three callers have been migrated to the new syntax. --- citadel/server/control.c | 14 +- citadel/server/euidindex.c | 11 +- .../autocompletion/serv_autocompletion.c | 143 +++++++----------- citadel/server/room_ops.c | 32 +++- citadel/server/room_ops.h | 1 + 5 files changed, 87 insertions(+), 114 deletions(-) diff --git a/citadel/server/control.c b/citadel/server/control.c index 27bc05b01..173ea61d4 100644 --- a/citadel/server/control.c +++ b/citadel/server/control.c @@ -14,6 +14,7 @@ #include "config.h" #include "citserver.h" #include "user_ops.h" +#include "room_ops.h" long control_highest_user = 0; @@ -47,7 +48,6 @@ struct cfh { // 2 = show inconsistencies but don't repair them, continue execution void control_find_highest(struct ctdlroom *qrbuf, void *data) { struct cfh *cfh = (struct cfh *)data; - struct cdbdata *cdbfr; long *msglist; int num_msgs=0; int c; @@ -57,13 +57,9 @@ void control_find_highest(struct ctdlroom *qrbuf, void *data) { } // Load the message list - cdbfr = cdb_fetch(CDB_MSGLISTS, &qrbuf->QRnumber, sizeof(long)); - if (cdbfr != NULL) { - msglist = (long *) cdbfr->ptr; - num_msgs = cdbfr->len / sizeof(long); - } - else { - return; // No messages at all? No further action. + num_msgs = CtdlFetchMsgList(qrbuf->QRnumber, &msglist); + if (num_msgs < 0) { + return; // No msglists record? No further action. } if (num_msgs > 0) { @@ -74,7 +70,7 @@ void control_find_highest(struct ctdlroom *qrbuf, void *data) { } } - cdb_free(cdbfr); + free(msglist); } diff --git a/citadel/server/euidindex.c b/citadel/server/euidindex.c index cf26193b6..14aafd1d6 100644 --- a/citadel/server/euidindex.c +++ b/citadel/server/euidindex.c @@ -166,7 +166,6 @@ void rebuild_euid_index(void) { void cmd_euid(char *cmdbuf) { char euid[256]; long msgnum; - struct cdbdata *cdbfr; long *msglist = NULL; int num_msgs = 0; int i; @@ -180,18 +179,16 @@ void cmd_euid(char *cmdbuf) { return; } - cdbfr = cdb_fetch(CDB_MSGLISTS, &CC->room.QRnumber, sizeof(long)); - if (cdbfr != NULL) { - num_msgs = cdbfr->len / sizeof(long); - msglist = (long *) cdbfr->ptr; + num_msgs = CtdlFetchMsgList(CC->room.QRnumber, &msglist); + if (num_msgs >= 0) { for (i = 0; i < num_msgs; ++i) { if (msglist[i] == msgnum) { - cdb_free(cdbfr); + free(msglist); cprintf("%d %ld\n", CIT_OK, msgnum); return; } } - cdb_free(cdbfr); + free(msglist); } cprintf("%d not found\n", ERROR + MESSAGE_NOT_FOUND); diff --git a/citadel/server/modules/autocompletion/serv_autocompletion.c b/citadel/server/modules/autocompletion/serv_autocompletion.c index fb225ddb4..3f025521f 100644 --- a/citadel/server/modules/autocompletion/serv_autocompletion.c +++ b/citadel/server/modules/autocompletion/serv_autocompletion.c @@ -1,27 +1,18 @@ -/* - * Autocompletion of email recipients, etc. - * - * Copyright (c) 1987-2022 by the citadel.org team - * - * This program is open source software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 3. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - +// Autocompletion of email recipients, etc. +// +// Copyright (c) 1987-2023 by the citadel.org team +// +// This program is open source software. Use, duplication, or disclosure +// are subject to the terms of the GNU General Public License version 3. #include "../../ctdl_module.h" #include "serv_autocompletion.h" #include "../../config.h" +#include "../../room_ops.h" -/* - * Convert a structured name into a friendly name. Caller must free the - * returned pointer. - */ +// Convert a structured name into a friendly name. Caller must free the +// returned pointer. char *n_to_fn(char *value) { char *nnn = NULL; int i; @@ -46,11 +37,7 @@ char *n_to_fn(char *value) { } - - -/* - * Back end for cmd_auto() - */ +// Back end for cmd_auto() void hunt_for_autocomplete(long msgnum, char *search_string) { struct CtdlMessage *msg; struct vCard *v; @@ -65,11 +52,9 @@ void hunt_for_autocomplete(long msgnum, char *search_string) { v = vcard_load(msg->cm_fields[eMesageText]); CM_Free(msg); - /* - * Try to match from a friendly name (the "fn" field). If there is - * a match, return the entry in the form of: - * Display Name - */ + // Try to match from a friendly name (the "fn" field). If there is + // a match, return the entry in the form of: + // Display Name value = vcard_get_prop(v, "fn", 0, 0, 0); if (value != NULL) if (bmstrcasestr(value, search_string)) { value2 = vcard_get_prop(v, "email", 1, 0, 0); @@ -79,11 +64,9 @@ void hunt_for_autocomplete(long msgnum, char *search_string) { return; } - /* - * Try to match from a structured name (the "n" field). If there is - * a match, return the entry in the form of: - * Display Name - */ + // Try to match from a structured name (the "n" field). If there is + // a match, return the entry in the form of: + // Display Name value = vcard_get_prop(v, "n", 0, 0, 0); if (value != NULL) if (bmstrcasestr(value, search_string)) { @@ -96,9 +79,7 @@ void hunt_for_autocomplete(long msgnum, char *search_string) { return; } - /* - * Try a partial match on all listed email addresses. - */ + // Try a partial match on all listed email addresses. i = 0; while (value = vcard_get_prop(v, "email", 1, i++, 0), value != NULL) { if (bmstrcasestr(value, search_string)) { @@ -109,7 +90,6 @@ void hunt_for_autocomplete(long msgnum, char *search_string) { nnn = n_to_fn(vcard_get_prop(v, "n", 0, 0, 0)); cprintf("%s <%s>\n", nnn, value); free(nnn); - } else { cprintf("%s\n", value); @@ -123,10 +103,7 @@ void hunt_for_autocomplete(long msgnum, char *search_string) { } - -/* - * Attempt to autocomplete an address based on a partial... - */ +// Attempt to autocomplete an address based on a partial... void cmd_auto(char *argbuf) { char hold_rm[ROOMNAMELEN]; char search_string[256]; @@ -134,7 +111,6 @@ void cmd_auto(char *argbuf) { int num_msgs = 0; long *fts_msgs = NULL; int fts_num_msgs = 0; - struct cdbdata *cdbfr; int r = 0; int i = 0; int j = 0; @@ -144,69 +120,56 @@ void cmd_auto(char *argbuf) { if (CtdlAccessCheck(ac_logged_in)) return; extract_token(search_string, argbuf, 0, '|', sizeof search_string); if (IsEmptyStr(search_string)) { - cprintf("%d You supplied an empty partial.\n", - ERROR + ILLEGAL_VALUE); + cprintf("%d You supplied an empty partial.\n", ERROR + ILLEGAL_VALUE); return; } - strcpy(hold_rm, CC->room.QRname); /* save current room */ + strcpy(hold_rm, CC->room.QRname); // save where we were so we can go back cprintf("%d try these:\n", LISTING_FOLLOWS); - /* - * Gather up message pointers in rooms containing vCards - */ + // Gather up message pointers in rooms containing vCards for (r=0; r < (sizeof(rooms_to_try) / sizeof(char *)); ++r) { if (CtdlGetRoom(&CC->room, rooms_to_try[r]) == 0) { - cdbfr = cdb_fetch(CDB_MSGLISTS, &CC->room.QRnumber, sizeof(long)); - if (cdbfr != NULL) { - msglist = realloc(msglist, (num_msgs * sizeof(long)) + cdbfr->len + 1); - memcpy(&msglist[num_msgs], cdbfr->ptr, cdbfr->len); - num_msgs += (cdbfr->len / sizeof(long)); - cdb_free(cdbfr); - } - } - } - - /* - * Search-reduce the results if we have the full text index available - */ - if (CtdlGetConfigInt("c_enable_fulltext")) { - CtdlModuleDoSearch(&fts_num_msgs, &fts_msgs, search_string, "fulltext"); - if (fts_msgs) { - for (i=0; iroom.QRnumber, &msglist); + + // Search-reduce the results if we have the full text index available + if (CtdlGetConfigInt("c_enable_fulltext")) { + CtdlModuleDoSearch(&fts_num_msgs, &fts_msgs, search_string, "fulltext"); + if (fts_msgs) { + for (i=0; i 0) for (i=0; i 0) for (i=0; iroom.QRname, hold_rm)) { - CtdlGetRoom(&CC->room, hold_rm); /* return to saved room */ + CtdlGetRoom(&CC->room, hold_rm); // return to saved room } if (msglist) { @@ -221,6 +184,6 @@ char *ctdl_module_init_autocompletion(void) { if (!threading) { CtdlRegisterProtoHook(cmd_auto, "AUTO", "Do recipient autocompletion"); } - /* return our module name for the log */ + // return our module name for the log return "autocompletion"; } diff --git a/citadel/server/room_ops.c b/citadel/server/room_ops.c index 1566e74b0..b8f8113d7 100644 --- a/citadel/server/room_ops.c +++ b/citadel/server/room_ops.c @@ -601,6 +601,29 @@ int CtdlIsNonEditable(struct ctdlroom *qrbuf) { } +// Retrieve a list of all messages (message numbers) in the specified room. +// Returns the number of messages in the room, allocates a pointer to the array and stuffs it in the supplied location. +// Caller must free that memory. +// Returns (-1) if error. +int CtdlFetchMsgList(long roomnum, long **msgs) { + int num_msgs = 0; + struct cdbdata *cdbfr; + + cdbfr = cdb_fetch(CDB_MSGLISTS, &CC->room.QRnumber, sizeof(long)); + if (cdbfr == NULL) { + syslog(LOG_ERR, "room_ops: no msglist for room %ld", roomnum); + *msgs = NULL; + return (-1); + } + + *msgs = malloc(cdbfr->len); + memcpy(*msgs, cdbfr->ptr, cdbfr->len); + num_msgs = cdbfr->len / sizeof(long); + cdb_free(cdbfr); + return(num_msgs); +} + + // Make the specified room the current room for this session. No validation // or access control is done here -- the caller should make sure that the // specified room exists and is ok to access. @@ -616,7 +639,6 @@ void CtdlUserGoto(char *where, int display_result, int transiently, int *retmsgs int raideflag; struct visit vbuf; char truncated_roomname[ROOMNAMELEN]; - struct cdbdata *cdbfr; long *msglist = NULL; int num_msgs = 0; unsigned int original_v_flags; @@ -667,13 +689,7 @@ void CtdlUserGoto(char *where, int display_result, int transiently, int *retmsgs info = 1; } - cdbfr = cdb_fetch(CDB_MSGLISTS, &CC->room.QRnumber, sizeof(long)); - if (cdbfr != NULL) { - msglist = (long *) cdbfr->ptr; - cdbfr->ptr = NULL; // CtdlUserGoto() now owns this memory (this requires attention if we move to LMDB) - num_msgs = cdbfr->len / sizeof(long); - cdb_free(cdbfr); - } + num_msgs = CtdlFetchMsgList(CC->room.QRnumber, &msglist); total_messages = 0; for (a=0; a