DRY fetch of msglists.
authorArt Cancro <ajc@citadel.org>
Wed, 16 Aug 2023 03:14:24 +0000 (18:14 -0900)
committerArt Cancro <ajc@citadel.org>
Wed, 16 Aug 2023 03:14:24 +0000 (18:14 -0900)
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
citadel/server/euidindex.c
citadel/server/modules/autocompletion/serv_autocompletion.c
citadel/server/room_ops.c
citadel/server/room_ops.h

index 27bc05b01e52ceded4715719327b59281520ecdd..173ea61d4ce6d76fbbe17424834e3ae423a267e9 100644 (file)
@@ -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);
 }
 
 
index cf26193b626c0f92de5f937dd88ee5b7dae441df..14aafd1d6ecd388f2fc0a3ab80ced745a2074897 100644 (file)
@@ -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);
index fb225ddb42ef05dc12d45aa4084a760654082df6..3f025521fe76b4250c3d8851ddf68671758a7ac5 100644 (file)
@@ -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 <user@domain.org>
-        */
+       // Try to match from a friendly name (the "fn" field).  If there is
+       // a match, return the entry in the form of:
+       //     Display Name <user@domain.org>
        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 <user@domain.org>
-        */
+       // Try to match from a structured name (the "n" field).  If there is
+       // a match, return the entry in the form of:
+       //     Display Name <user@domain.org>
        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; i<num_msgs; ++i) {
-                               search_match = 0;
-                               for (j=0; j<fts_num_msgs; ++j) {
-                                       if (msglist[i] == fts_msgs[j]) {
-                                               search_match = 1;
-                                               j = fts_num_msgs + 1;   /* end the search */
+                       num_msgs = CtdlFetchMsgList(CC->room.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<num_msgs; ++i) {
+                                               search_match = 0;
+                                               for (j=0; j<fts_num_msgs; ++j) {
+                                                       if (msglist[i] == fts_msgs[j]) {
+                                                               search_match = 1;
+                                                               j = fts_num_msgs + 1;   // end the search 
+                                                       }
+                                               }
+                                               if (!search_match) {
+                                                       msglist[i] = 0;                 // invalidate this result
+                                               }
                                        }
+                                       free(fts_msgs);
                                }
-                               if (!search_match) {
-                                       msglist[i] = 0;         /* invalidate this result */
+                               else {
+                                       // If no results, invalidate the whole list
+                                       free(msglist);
+                                       msglist = NULL;
+                                       num_msgs = 0;
+                               }
+                       }
+               
+                       // Now output the ones that look interesting
+                       if (num_msgs > 0) for (i=0; i<num_msgs; ++i) {
+                               if (msglist[i] != 0) {
+                                       hunt_for_autocomplete(msglist[i], search_string);
                                }
                        }
-                       free(fts_msgs);
-               }
-               else {
-                       /* If no results, invalidate the whole list */
-                       free(msglist);
-                       msglist = NULL;
-                       num_msgs = 0;
-               }
-       }
-
-       /*
-        * Now output the ones that look interesting
-        */
-       if (num_msgs > 0) for (i=0; i<num_msgs; ++i) {
-               if (msglist[i] != 0) {
-                       hunt_for_autocomplete(msglist[i], search_string);
                }
        }
        
        cprintf("000\n");
        if (strcmp(CC->room.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";
 }
index 1566e74b0a85261426a9bd1994816048a0beca8b..b8f8113d73777329353f4d69237379088cc5dcc5 100644 (file)
@@ -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<num_msgs; ++a) {
index ba1aaf644e0b602ae33e20b553114123c3b40b5e..f09a8cf3e94afc78df9a67e3d18233712f1b305c 100644 (file)
@@ -5,6 +5,7 @@ void b_putroom(struct ctdlroom *qrbuf, char *room_name);
 void b_deleteroom(char *);
 void lgetfloor (struct floor *flbuf, int floor_num);
 void lputfloor (struct floor *flbuf, int floor_num);
+int CtdlFetchMsgList(long roomnum, long **msgs);
 int sort_msglist (long int *listptrs, int oldcount);
 void list_roomname(struct ctdlroom *qrbuf, int ra, int current_view, int default_view);
 void convert_room_name_macros(char *towhere, size_t maxlen);