berkeley_db.c: improvements to transactional data store
authorArt Cancro <ajc@citadel.org>
Sat, 26 Aug 2023 23:19:54 +0000 (19:19 -0400)
committerArt Cancro <ajc@citadel.org>
Sat, 26 Aug 2023 23:19:54 +0000 (19:19 -0400)
https://docs.oracle.com/cd/E17276_01/html/programmer_reference/transapp_env_open.html

The use of the DB_RECOVER flag to dbenv->open() appears to do more than just try to
recover the environment upon startup.  It also creates __db.00X files, which are
perhaps holding the environment???   In any case it seems to be more solid this way.
I am also removing the use of any "private" transactions, since they are no longer
needed.  Operations which write to the database check to see if the thread's dedicated
transaction pointer is in effect, maintaining that transaction if it is, and creating
a transaction in the same pointer otherwise.

There is a "double free" bug somewhere in here and I'm still trying to figure out what
conditions cause it to happen.

citadel/server/backends/berkeley_db/berkeley_db.c
citadel/server/modules/expire/serv_expire.c
citadel/server/modules/fulltext/serv_fulltext.c
citadel/server/msgbase.c

index 147446f6e0038452b74e885d537ed53bbdd0392f..4d77a0b1483b33aba78e8a21ce7aa1a6a15b9be6 100644 (file)
@@ -112,19 +112,6 @@ static void bdb_txcommit(DB_TXN *tid) {
 }
 
 
-// Wrapper for txn_begin() that logs/aborts on error.  Not part of the backend API.
-static void bdb_txbegin(DB_TXN **tid) {
-       int ret;
-
-       ret = bdb_env->txn_begin(bdb_env, NULL, tid, 0);
-
-       if (ret) {
-               syslog(LOG_ERR, "bdb: txn_begin: %s", db_strerror(ret));
-               bdb_abort();
-       }
-}
-
-
 // Panic callback for Berkeley DB.  Not part of the backend API.
 static void bdb_dbpanic(DB_ENV *env, int errval) {
        syslog(LOG_ERR, "bdb: PANIC: %s", db_strerror(errval));
@@ -166,6 +153,48 @@ void bdb_check_handles(void) {
 }
 
 
+// Transaction-based stuff.  I'm writing this as I bake cookies...
+void bdb_begin_transaction(void) {
+       int ret;
+       bdb_bailIfCursor(TSD->cursors, "can't begin transaction during r/o cursor");
+
+       if (TSD->tid != NULL) {
+               syslog(LOG_ERR, "bdb: bdb_begin_transaction: ERROR: nested transaction");
+               bdb_abort();
+       }
+
+       ret = bdb_env->txn_begin(bdb_env, NULL, &TSD->tid, 0);
+       if (ret) {
+               syslog(LOG_ERR, "bdb: bdb_begin_transaction: %s", db_strerror(ret));
+               bdb_abort();
+       }
+}
+
+
+// ...and the cookies are cursed.
+void bdb_end_transaction(void) {
+       int i;
+
+       for (i = 0; i < MAXCDB; i++) {
+               if (TSD->cursors[i] != NULL) {
+                       syslog(LOG_WARNING, "bdb: bdb_end_transaction: WARNING: cursor %d still open at transaction end", i);
+                       bdb_cclose(TSD->cursors[i]);
+                       TSD->cursors[i] = NULL;
+               }
+       }
+
+       if (TSD->tid == NULL) {
+               syslog(LOG_ERR, "bdb: bdb_end_transaction: ERROR: bdb_txcommit(NULL) !!");
+               bdb_abort();
+       }
+       else {
+               bdb_txcommit(TSD->tid);
+       }
+
+       TSD->tid = NULL;
+}
+
+
 // Request a checkpoint of the database.  Called once per minute by the thread manager.
 void bdb_checkpoint(void) {
        int ret;
@@ -226,42 +255,11 @@ void bdb_open_databases(void) {
        bdb_env->set_verbose(bdb_env, DB_VERB_DEADLOCK, 1);
        bdb_env->set_verbose(bdb_env, DB_VERB_RECOVERY, 1);
 
-       // We want to specify the shared memory buffer pool cachesize, but everything else is the default.
-       // 2023aug21 ajc: the third argument is zero, so this never did anything
-       // ret = bdb_env->set_cachesize(bdb_env, 0, 64 * 1024, 0);
-       // if (ret) {
-               // syslog(LOG_ERR, "bdb: set_cachesize: %s", db_strerror(ret));
-               // bdb_env->close(bdb_env, 0);
-               // syslog(LOG_ERR, "bdb: exit code %d", ret);
-               // exit(CTDLEXIT_DB);
-       // }
-
-       // This appears to do nothing over and above the default
-       //if ((ret = bdb_env->set_lk_detect(bdb_env, DB_LOCK_DEFAULT))) {
-               //syslog(LOG_ERR, "bdb: set_lk_detect: %s", db_strerror(ret));
-               //bdb_env->close(bdb_env, 0);
-               //syslog(LOG_ERR, "bdb: exit code %d", ret);
-               //exit(CTDLEXIT_DB);
-       //}
-
-       flags = DB_CREATE | DB_INIT_MPOOL | DB_PRIVATE | DB_INIT_TXN | /*DB_INIT_LOCK |*/ DB_THREAD | DB_INIT_LOG;
+       flags = DB_CREATE | DB_INIT_LOCK | DB_INIT_LOG | DB_INIT_MPOOL | DB_INIT_TXN | DB_RECOVER | DB_THREAD;
        syslog(LOG_DEBUG, "bdb: bdb_env->open(bdb_env, %s, %d, 0)", ctdl_db_dir, flags);
        ret = bdb_env->open(bdb_env, ctdl_db_dir, flags, 0);                            // try opening the database cleanly
-       if (ret == DB_RUNRECOVERY) {
-               syslog(LOG_ERR, "bdb: bdb_env->open: %s", db_strerror(ret));
-               syslog(LOG_ERR, "bdb: attempting recovery...");
-               flags |= DB_RECOVER;
-               ret = bdb_env->open(bdb_env, ctdl_db_dir, flags, 0);                    // try recovery
-       }
-       if (ret == DB_RUNRECOVERY) {
-               syslog(LOG_ERR, "bdb: bdb_env->open: %s", db_strerror(ret));
-               syslog(LOG_ERR, "bdb: attempting catastrophic recovery...");
-               flags &= ~DB_RECOVER;
-               flags |= DB_RECOVER_FATAL;
-               ret = bdb_env->open(bdb_env, ctdl_db_dir, flags, 0);                    // try catastrophic recovery
-       }
        if (ret) {
-               syslog(LOG_ERR, "bdb: bdb_env->open: %s", db_strerror(ret));
+               syslog(LOG_ERR, "bdb: bdb_env->open: %s: %s", ctdl_db_dir, db_strerror(ret));
                bdb_env->close(bdb_env, 0);
                syslog(LOG_ERR, "bdb: exit code %d", ret);
                exit(CTDLEXIT_DB);
@@ -386,6 +384,7 @@ int bdb_store(int cdb, const void *ckey, int ckeylen, void *cdata, int cdatalen)
        int compressing = 0;
        size_t buffer_len = 0;
        uLongf destLen = 0;
+       int existing_txn = 0;                   // set to nonzero if we are already inside a transaction
 
        memset(&dkey, 0, sizeof(DBT));
        memset(&ddata, 0, sizeof(DBT));
@@ -414,49 +413,33 @@ int bdb_store(int cdb, const void *ckey, int ckeylen, void *cdata, int cdatalen)
        }
 
        if (TSD->tid != NULL) {
-               ret = bdb_table[cdb]->put(bdb_table[cdb],       // db
-                                   TSD->tid,                   // transaction ID
-                                   &dkey,                      // key
-                                   &ddata,                     // data
-                                   0                           // flags
-               );
-               if (ret) {
-                       syslog(LOG_ERR, "bdb: bdb_store(%d): %s", cdb, db_strerror(ret));
-                       bdb_abort();
-               }
-               if (compressing) {
-                       free(compressed_data);
-               }
-               return ret;
+               existing_txn = 1;
        }
-       else {
-               bdb_bailIfCursor(TSD->cursors, "attempt to write during r/o cursor");
-
-             retry:
-               bdb_txbegin(&tid);
-
-               if ((ret = bdb_table[cdb]->put(bdb_table[cdb],  // db
-                                        tid,                   // transaction ID
-                                        &dkey,                 // key
-                                        &ddata,                // data
-                                        0))) {                 // flags
-                       if (ret == DB_LOCK_DEADLOCK) {
-                               bdb_txabort(tid);
-                               goto retry;
-                       }
-                       else {
-                               syslog(LOG_ERR, "bdb: bdb_store(%d): %s", cdb, db_strerror(ret));
-                               bdb_abort();
-                       }
-               }
-               else {
-                       bdb_txcommit(tid);
-                       if (compressing) {
-                               free(compressed_data);
-                       }
-                       return ret;
-               }
+
+       if (!existing_txn) {                            // If we're not already inside a transaction,
+               bdb_begin_transaction();                // create our own for this operation.
+       }
+
+       ret = bdb_table[cdb]->put(bdb_table[cdb],       // db
+                           TSD->tid,                   // transaction ID
+                           &dkey,                      // key
+                           &ddata,                     // data
+                           0                           // flags
+       );
+
+       if (ret) {
+               syslog(LOG_ERR, "bdb: bdb_store(%02x): %s", cdb, db_strerror(ret));
+               bdb_abort();
+       }
+
+       if (!existing_txn) {
+               bdb_end_transaction();
+       }
+
+       if (compressing) {
+               free(compressed_data);
        }
+
        return ret;
 }
 
@@ -464,68 +447,39 @@ int bdb_store(int cdb, const void *ckey, int ckeylen, void *cdata, int cdatalen)
 // Delete a piece of data.  Returns 0 if the operation was successful.
 int bdb_delete(int cdb, void *key, int keylen) {
        DBT dkey;
-       DB_TXN *tid;
        int ret;
+       int existing_txn = 0;                           // set to nonzero if we are already inside a transaction
 
        memset(&dkey, 0, sizeof dkey);
        dkey.size = keylen;
        dkey.data = key;
 
        if (TSD->tid != NULL) {
-               ret = bdb_table[cdb]->del(bdb_table[cdb], TSD->tid, &dkey, 0);
-               if (ret) {
-                       syslog(LOG_ERR, "bdb: bdb_delete(%d): %s", cdb, db_strerror(ret));
-                       if (ret != DB_NOTFOUND) {
-                               bdb_abort();
-                       }
-               }
+               existing_txn = 1;
        }
-       else {
-               bdb_bailIfCursor(TSD->cursors, "attempt to delete during r/o cursor");
 
-             retry:
-               bdb_txbegin(&tid);
-
-               if ((ret = bdb_table[cdb]->del(bdb_table[cdb], tid, &dkey, 0)) && ret != DB_NOTFOUND) {
-                       if (ret == DB_LOCK_DEADLOCK) {
-                               bdb_txabort(tid);
-                               goto retry;
-                       }
-                       else {
-                               syslog(LOG_ERR, "bdb: bdb_delete(%d): %s", cdb, db_strerror(ret));
-                               bdb_abort();
-                       }
-               }
-               else {
-                       bdb_txcommit(tid);
-               }
+       if (!existing_txn) {                            // If we're not already inside a transaction,
+               bdb_begin_transaction();                // create our own for this operation.
        }
-       return ret;
-}
 
-
-static DBC *bdb_localcursor(int cdb) {
-       int ret;
-       DBC *curs;
-
-       if (TSD->cursors[cdb] == NULL) {
-               ret = bdb_table[cdb]->cursor(bdb_table[cdb], TSD->tid, &curs, 0);
-       }
-       else {
-               ret = TSD->cursors[cdb]->c_dup(TSD->cursors[cdb], &curs, DB_POSITION);
+       ret = bdb_table[cdb]->del(bdb_table[cdb], TSD->tid, &dkey, 0);
+       if (ret) {
+               if (ret != DB_NOTFOUND) {
+                       syslog(LOG_ERR, "bdb: bdb_delete(%02x): %s", cdb, db_strerror(ret));
+                       bdb_abort();
+               }
        }
 
-       if (ret) {
-               syslog(LOG_ERR, "bdb: bdb_localcursor: %s", db_strerror(ret));
-               bdb_abort();
+       if (!existing_txn) {
+               bdb_end_transaction();                  // Only end the transaction if we began it.
        }
 
-       return curs;
+       return ret;
 }
 
 
 // Fetch a piece of data.  Returns a "struct cdbdata"
-// cdbdata.len will be 0 if the item is not found.
+// If the item is not found, the pointer will be NULL.
 struct cdbdata bdb_fetch(int cdb, const void *key, int keylen) {
 
        struct cdbdata returned_data;
@@ -542,20 +496,8 @@ struct cdbdata bdb_fetch(int cdb, const void *key, int keylen) {
        dkey.size = keylen;
        dkey.data = (void *) key;
 
-       if (TSD->tid != NULL) {
-               TSD->dbdata[cdb].flags = DB_DBT_REALLOC;
-               ret = bdb_table[cdb]->get(bdb_table[cdb], TSD->tid, &dkey, &TSD->dbdata[cdb], 0);
-       }
-       else {
-               DBC *curs;
-
-               do {
-                       TSD->dbdata[cdb].flags = DB_DBT_REALLOC;
-                       curs = bdb_localcursor(cdb);
-                       ret = curs->c_get(curs, &dkey, &TSD->dbdata[cdb], DB_SET);
-                       bdb_cclose(curs);
-               } while (ret == DB_LOCK_DEADLOCK);
-       }
+       TSD->dbdata[cdb].flags = DB_DBT_REALLOC;
+       ret = bdb_table[cdb]->get(bdb_table[cdb], TSD->tid, &dkey, &TSD->dbdata[cdb], 0);
 
        if ((ret != 0) && (ret != DB_NOTFOUND)) {
                syslog(LOG_ERR, "bdb: bdb_fetch(%d): %s", cdb, db_strerror(ret));
@@ -635,42 +577,6 @@ struct cdbkeyval bdb_next_item(int cdb) {
 }
 
 
-// Transaction-based stuff.  I'm writing this as I bake cookies...
-void bdb_begin_transaction(void) {
-       bdb_bailIfCursor(TSD->cursors, "can't begin transaction during r/o cursor");
-
-       if (TSD->tid != NULL) {
-               syslog(LOG_ERR, "bdb: bdb_begin_transaction: ERROR: nested transaction");
-               bdb_abort();
-       }
-
-       bdb_txbegin(&TSD->tid);
-}
-
-
-void bdb_end_transaction(void) {
-       int i;
-
-       for (i = 0; i < MAXCDB; i++) {
-               if (TSD->cursors[i] != NULL) {
-                       syslog(LOG_WARNING, "bdb: bdb_end_transaction: WARNING: cursor %d still open at transaction end", i);
-                       bdb_cclose(TSD->cursors[i]);
-                       TSD->cursors[i] = NULL;
-               }
-       }
-
-       if (TSD->tid == NULL) {
-               syslog(LOG_ERR, "bdb: bdb_end_transaction: ERROR: bdb_txcommit(NULL) !!");
-               bdb_abort();
-       }
-       else {
-               bdb_txcommit(TSD->tid);
-       }
-
-       TSD->tid = NULL;
-}
-
-
 // Truncate (delete every record)
 void bdb_trunc(int cdb) {
        int ret;
@@ -680,27 +586,21 @@ void bdb_trunc(int cdb) {
                syslog(LOG_ERR, "bdb: bdb_trunc must not be called in a transaction.");
                bdb_abort();
        }
-       else {
-               bdb_bailIfCursor(TSD->cursors, "attempt to write during r/o cursor");
-
-             retry:
 
-               if ((ret = bdb_table[cdb]->truncate(bdb_table[cdb],     // db
-                                             NULL,                     // transaction ID
-                                             &count,                   // #rows deleted
-                                             0))) {                    // flags
-                       if (ret == DB_LOCK_DEADLOCK) {
-                               goto retry;
-                       }
-                       else {
-                               syslog(LOG_ERR, "bdb: bdb_truncate(%d): %s", cdb, db_strerror(ret));
-                               if (ret == ENOMEM) {
-                                       syslog(LOG_ERR, "bdb: You may need to tune your database; please read http://www.citadel.org for more information.");
-                               }
-                               exit(CTDLEXIT_DB);
-                       }
+       bdb_begin_transaction();                                // create our own transaction for this operation.
+       ret = bdb_table[cdb]->truncate(bdb_table[cdb],          // db
+                                     NULL,                     // transaction ID
+                                     &count,                   // #rows deleted
+                                     0);                       // flags
+                                                               //
+       if (ret) {
+               syslog(LOG_ERR, "bdb: bdb_truncate(%d): %s", cdb, db_strerror(ret));
+               if (ret == ENOMEM) {
+                       syslog(LOG_ERR, "bdb: You may need to tune your database; please read http://www.citadel.org for more information.");
                }
+               exit(CTDLEXIT_DB);
        }
+       bdb_end_transaction();
 }
 
 
index 876edc9fa4c8b167ee1960be256ce246a2d2e390..f360804a2639c2068b4d0cf7a4aad7a9d221f628 100644 (file)
@@ -552,6 +552,7 @@ int PurgeVisits(void) {
        }
 
        // Now delete every visit on the purged list
+       cdb_begin_transaction();
        while (VisitPurgeList != NULL) {
                IndexLen = GenerateRelationshipIndex(IndexBuf,
                                VisitPurgeList->vp_roomnum,
@@ -563,6 +564,7 @@ int PurgeVisits(void) {
                VisitPurgeList = vptr;
                ++purged;
        }
+       cdb_end_transaction();
 
        return(purged);
 }
@@ -599,10 +601,12 @@ int PurgeUseTable(StrBuf *ErrMsg) {
        // Phase 2: delete the records
        syslog(LOG_DEBUG, "Purge use table: phase 2");
        int i;
+       cdb_begin_transaction();
        for (i=0; i<purged; ++i) {
                struct UseTable *u = (struct UseTable *)array_get_element_at(purge_list, i);
                cdb_delete(CDB_USETABLE, &u->hash, sizeof(int));
        }
+       cdb_end_transaction();
        array_free(purge_list);
 
        syslog(LOG_DEBUG, "Purge use table: finished (purged %d of %d records)", purged, total);
@@ -647,6 +651,7 @@ int PurgeEuidIndexTable(void) {
 
        // Phase 2: delete the records
        syslog(LOG_DEBUG, "Purge euid index: phase 2");
+       cdb_begin_transaction();
        while (el != NULL) {
                cdb_delete(CDB_EUIDINDEX, el->ep_key, el->ep_keylen);
                free(el->ep_key);
@@ -654,6 +659,7 @@ int PurgeEuidIndexTable(void) {
                free(el);
                el = eptr;
        }
+       cdb_end_transaction();
 
        syslog(LOG_DEBUG, "Purge euid index: finished (purged %d records)", purged);
        return(purged);
index 99d80cbba1731744fe9368e5912735241d6232ee..b80ade388ad29a4594298a2ba7d153cc224e0293 100644 (file)
@@ -115,7 +115,7 @@ void ft_index_message(long msgnum, int op) {
        wordbreaker(txt, &num_tokens, &tokens);
        free(txt);
 
-       syslog(LOG_DEBUG, "fulltext: indexing message %ld [%d tokens]", msgnum, num_tokens);
+       syslog(LOG_DEBUG, "fulltext: %sindexing message %ld [%d tokens]", (op ? "" : "de"), msgnum, num_tokens);
        if (num_tokens > 0) {
                for (i=0; i<num_tokens; ++i) {
 
index d277f070d38f943e53b3195fdefd9c3f356bab61..c6c40a56010e0ba36f8bb949d16fe38eb378408f 100644 (file)
@@ -656,25 +656,21 @@ int CtdlForEachMessage(int mode, long ref, char *search_string,
                return 0;       // No messages at all?  No further action.
        }
 
-       /*
-        * Now begin the traversal.
-        */
+       // Now begin the traversal.
        if (num_msgs > 0) for (a = 0; a < num_msgs; ++a) {
 
-               /* If the caller is looking for a specific MIME type, filter
-                * out all messages which are not of the type requested.
-                */
+               // If the caller is looking for a specific MIME type, filter
+               // out all messages which are not of the type requested.
                if ((content_type != NULL) && (!IsEmptyStr(content_type))) {
 
-                       /* This call to GetMetaData() sits inside this loop
-                        * so that we only do the extra database read per msg
-                        * if we need to.  Doing the extra read all the time
-                        * really kills the server.  If we ever need to use
-                        * metadata for another search criterion, we need to
-                        * move the read somewhere else -- but still be smart
-                        * enough to only do the read if the caller has
-                        * specified something that will need it.
-                        */
+                       // This call to GetMetaData() sits inside this loop
+                       // so that we only do the extra database read per msg
+                       // if we need to.  Doing the extra read all the time
+                       // really kills the server.  If we ever need to use
+                       // metadata for another search criterion, we need to
+                       // move the read somewhere else -- but still be smart
+                       // enough to only do the read if the caller has
+                       // specified something that will need it.
                        if (server_shutting_down) {
                                if (need_to_free_re) regfree(&re);
                                free(msglist);
@@ -682,7 +678,6 @@ int CtdlForEachMessage(int mode, long ref, char *search_string,
                        }
                        GetMetaData(&smi, msglist[a]);
 
-                       /* if (strcasecmp(smi.meta_content_type, content_type)) { old non-regex way */
                        if (regexec(&re, smi.meta_content_type, 1, &pm, 0) != 0) {
                                msglist[a] = 0L;
                        }
@@ -691,9 +686,7 @@ int CtdlForEachMessage(int mode, long ref, char *search_string,
 
        num_msgs = sort_msglist(msglist, num_msgs);
 
-       /* If a template was supplied, filter out the messages which
-        * don't match.  (This could induce some delays!)
-        */
+       // If a template was supplied, filter out the messages which don't match.  (This could induce some delays!)
        if (num_msgs > 0) {
                if (compare != NULL) {
                        for (a = 0; a < num_msgs; ++a) {
@@ -3331,11 +3324,8 @@ void PutMetaData(struct MetaData *smibuf)
 }
 
 
-/*
- * Convenience function to process a big block of AdjRefCount() operations
- */
-void AdjRefCountList(long *msgnum, long nmsg, int incr)
-{
+// Convenience function to process a big block of AdjRefCount() operations
+void AdjRefCountList(long *msgnum, long nmsg, int incr) {
        long i;
 
        for (i = 0; i < nmsg; i++) {
@@ -3344,18 +3334,15 @@ void AdjRefCountList(long *msgnum, long nmsg, int incr)
 }
 
 
-/*
- * AdjRefCount - adjust the reference count for a message.  We need to delete from disk any message whose reference count reaches zero.
- */
-void AdjRefCount(long msgnum, int incr)
-{
+// AdjRefCount - adjust the reference count for a message.
+// We need to delete from disk any message whose reference count reaches zero.
+void AdjRefCount(long msgnum, int incr) {
        struct MetaData smi;
        long delnum;
 
-       /* This is a *tight* critical section; please keep it that way, as
-        * it may get called while nested in other critical sections.  
-        * Complicating this any further will surely cause deadlock!
-        */
+       // This is a *tight* critical section; please keep it that way, as
+       // it may get called while nested in other critical sections.  
+       // Complicating this any further will surely cause deadlock!
        begin_critical_section(S_SUPPMSGMAIN);
        GetMetaData(&smi, msgnum);
        smi.meta_refcount += incr;
@@ -3363,38 +3350,34 @@ void AdjRefCount(long msgnum, int incr)
        end_critical_section(S_SUPPMSGMAIN);
        syslog(LOG_DEBUG, "msgbase: AdjRefCount() msg %ld ref count delta %+d, is now %d", msgnum, incr, smi.meta_refcount);
 
-       /* If the reference count is now zero, delete both the message and its metadata record.
-        */
+       // If the reference count is now zero, delete both the message and its metadata record.
        if (smi.meta_refcount == 0) {
                syslog(LOG_DEBUG, "msgbase: deleting message <%ld>", msgnum);
                
-               /* Call delete hooks with NULL room to show it has gone altogether */
+               // Call delete hooks with NULL room to show it has gone altogether
                PerformDeleteHooks(NULL, msgnum);
 
-               /* Remove from message base */
+               // Remove from message base
                delnum = msgnum;
                cdb_delete(CDB_MSGMAIN, &delnum, (int)sizeof(long));
-               cdb_delete(CDB_BIGMSGS, &delnum, (int)sizeof(long));
+               cdb_delete(CDB_BIGMSGS, &delnum, (int)sizeof(long));    // There might not be a bigmsgs.  Not an error.
 
-               /* Remove metadata record */
+               // Remove metadata record
                delnum = (0L - msgnum);
                cdb_delete(CDB_MSGMAIN, &delnum, (int)sizeof(long));
        }
 }
 
 
-/*
- * Write a generic object to this room
- *
- * Returns the message number of the written object, in case you need it.
- */
-long CtdlWriteObject(char *req_room,                   /* Room to stuff it in */
-                    char *content_type,                /* MIME type of this object */
-                    char *raw_message,                 /* Data to be written */
-                    off_t raw_length,                  /* Size of raw_message */
-                    struct ctdluser *is_mailbox,       /* Mailbox room? */
-                    int is_binary,                     /* Is encoding necessary? */
-                    unsigned int flags                 /* Internal save flags */
+// Write a generic object to this room
+// Returns the message number of the written object, in case you need it.
+long CtdlWriteObject(char *req_room,                   // Room to stuff it in
+                    char *content_type,                // MIME type of this object
+                    char *raw_message,                 // Data to be written
+                    off_t raw_length,                  // Size of raw_message
+                    struct ctdluser *is_mailbox,       // Mailbox room?
+                    int is_binary,                     // Is encoding necessary?
+                    unsigned int flags                 // Internal save flags
 ) {
        struct ctdlroom qrbuf;
        char roomname[ROOMNAMELEN];