Generate wiki diffs using a temp file instead of a big string of realloc() calls.

Noted a block of code in which we are smashing the stack and need to fix.

diff --git a/citadel/context.c b/citadel/context.c
index d890e1c13f423785e762cbb7def87930bc3d2a19..c77f3cdd2b007eb4fa8c5425ef3b0c63a1d3d16e 100644 (file)
--- a/citadel/context.c
+++ b/citadel/context.c
@@ -359,9 +359,9 @@ void RemoveContext (CitContext *con)
 
        /* 
         * If the client is still connected, blow 'em away. 
-        * if the socket is 0, its already gone or was never there.
+        * if the socket is 0 or -1, its already gone or was never there.
         */
-       if (con->client_socket != 0)
+       if (con->client_socket > 0)
        {
                syslog(LOG_NOTICE, "Closing socket %d\n", con->client_socket);
                close(con->client_socket);

diff --git a/citadel/modules/wiki/serv_wiki.c b/citadel/modules/wiki/serv_wiki.c
index 49a039809f6a130237c845fc5e63a6e9826fc5d7..62b2132d521d217c38037cc450a768e9c2d9bc11 100644 (file)
--- a/citadel/modules/wiki/serv_wiki.c
+++ b/citadel/modules/wiki/serv_wiki.c
@@ -83,6 +83,7 @@ int wiki_upload_beforesave(struct CtdlMessage *msg) {
        struct CtdlMessage *history_msg = NULL;
        char diff_old_filename[PATH_MAX];
        char diff_new_filename[PATH_MAX];
+       char diff_out_filename[PATH_MAX];
        char diff_cmd[PATH_MAX];
        FILE *fp;
        int rv;
@@ -90,7 +91,6 @@ int wiki_upload_beforesave(struct CtdlMessage *msg) {
        char boundary[256];
        char prefixed_boundary[258];
        char buf[1024];
-       int nbytes = 0;
        char *diffbuf = NULL;
        size_t diffbuf_len = 0;
        char *ptr = NULL;
@@ -154,6 +154,7 @@ int wiki_upload_beforesave(struct CtdlMessage *msg) {
         */
        CtdlMakeTempFileName(diff_old_filename, sizeof diff_old_filename);
        CtdlMakeTempFileName(diff_new_filename, sizeof diff_new_filename);
+       CtdlMakeTempFileName(diff_out_filename, sizeof diff_out_filename);
 
        if (old_msg != NULL) {
                fp = fopen(diff_old_filename, "w");
@@ -166,29 +167,37 @@ int wiki_upload_beforesave(struct CtdlMessage *msg) {
        rv = fwrite(msg->cm_fields['M'], strlen(msg->cm_fields['M']), 1, fp);
        fclose(fp);
 
-       diffbuf_len = 0;
-       diffbuf = NULL;
        snprintf(diff_cmd, sizeof diff_cmd,
-               "diff -u %s %s",
+               "diff -u %s %s >%s",
                diff_new_filename,
-               ((old_msg != NULL) ? diff_old_filename : "/dev/null")
+               ((old_msg != NULL) ? diff_old_filename : "/dev/null"),
+               diff_out_filename
        );
-       fp = popen(diff_cmd, "r");
+       syslog(LOG_DEBUG, "diff cmd: %s", diff_cmd);
+       rv = system(diff_cmd);
+       syslog(LOG_DEBUG, "diff cmd returned %d", rv);
+
+       diffbuf_len = 0;
+       diffbuf = NULL;
+       fp = fopen(diff_out_filename, "r");
+       if (fp == NULL) {
+               fp = fopen("/dev/null", "r");
+       }
        if (fp != NULL) {
-               do {
-                       diffbuf = realloc(diffbuf, diffbuf_len + 1025);
-                       nbytes = fread(&diffbuf[diffbuf_len], 1, 1024, fp);
-                       diffbuf_len += nbytes;
-               } while (nbytes == 1024);
+               fseek(fp, 0L, SEEK_END);
+               diffbuf_len = ftell(fp);
+               fseek(fp, 0L, SEEK_SET);
+               diffbuf = malloc(diffbuf_len + 1);
+               fread(diffbuf, diffbuf_len, 1, fp);
                diffbuf[diffbuf_len] = 0;
-               if (pclose(fp) != 0) {
-                       syslog(LOG_ERR, "pclose() returned an error - diff failed\n");
-               }
+               fclose(fp);
        }
-       syslog(LOG_DEBUG, "diff length is %d bytes\n", diffbuf_len);
+
+       syslog(LOG_DEBUG, "diff length is %d bytes", diffbuf_len);
 
        unlink(diff_old_filename);
        unlink(diff_new_filename);
+       unlink(diff_out_filename);
 
        /* Determine whether this was a bogus (empty) edit */
        if ((diffbuf_len = 0) && (diffbuf != NULL)) {
@@ -272,6 +281,10 @@ int wiki_upload_beforesave(struct CtdlMessage *msg) {
                }
        } while ( (IsEmptyStr(boundary)) && (*ptr != 0) );
 
+
+       /****************** STACK SMASH IS SOMEWHERE BELOW THIS LINE **************/
+
+
        /* Now look for the first boundary.  That is where we need to insert our fun.
         */
        if (!IsEmptyStr(boundary)) {
@@ -311,6 +324,9 @@ int wiki_upload_beforesave(struct CtdlMessage *msg) {
                }
 
                history_msg->cm_fields['T'] = realloc(history_msg->cm_fields['T'], 32);
+               if (history_msg->cm_fields['T'] == NULL) {
+                       syslog(LOG_EMERG, "*** REALLOC FAILED *** %s", strerror(errno));
+               }
                snprintf(history_msg->cm_fields['T'], 32, "%ld", time(NULL));
        
                CtdlSubmitMsg(history_msg, NULL, "", 0);
@@ -319,6 +335,10 @@ int wiki_upload_beforesave(struct CtdlMessage *msg) {
                syslog(LOG_ALERT, "Empty boundary string in history message.  No history!\n");
        }
 
+
+       /****************** STACK SMASH IS SOMEWHERE BELOW THIS LINE **************/
+
+
        free(diffbuf);
        free(history_msg);
        return(0);

diff --git a/citadel/serv_extensions.c b/citadel/serv_extensions.c
index 33c83f73887f9003c19a288aa036daf88884e5a1..76dbacb4a028ebe476ceda0e6abab84ddf09088a 100644 (file)
--- a/citadel/serv_extensions.c
+++ b/citadel/serv_extensions.c
@@ -969,8 +969,7 @@ int PerformMessageHooks(struct CtdlMessage *msg, int EventType)
 
        /* Other code may elect to protect this message from server-side
         * handlers; if this is the case, don't do anything.
-       syslog(LOG_DEBUG, "** Event type is %d, flags are %d\n",
-               EventType, msg->cm_flags);
+       syslog(LOG_DEBUG, "** Event type is %d, flags are %d\n", EventType, msg->cm_flags);
         */
        if (msg->cm_flags & CM_SKIP_HOOKS) {
                syslog(LOG_DEBUG, "Skipping hooks\n");
@@ -979,10 +978,13 @@ int PerformMessageHooks(struct CtdlMessage *msg, int EventType)
 
        /* Otherwise, run all the hooks appropriate to this event type.
         */
+       int num_hooks_processed = 0;
        for (fcn = MessageHookTable; fcn != NULL; fcn = fcn->next) {
                if (fcn->eventtype == EventType) {
-                       total_retval = total_retval +
-                               (*fcn->h_function_pointer) (msg);
+                       total_retval = total_retval + (*fcn->h_function_pointer) (msg);
+                       syslog(LOG_DEBUG, "%d hooks completed, total_retval=%d",
+                               ++num_hooks_processed, total_retval
+                       );
                }
        }