<<< Date Index >>>     <<< Thread Index >>>

[PATCH] memory leak in mbox.c



Hi,

while trying to reproduce PR/2172 (which failed), I noticed a small memory leak in case of fatal errors for syncing mbox folders.

With another check for ctx->path!=NULL, the attached patch doesn't leak oldOffset and newOffset from that routine any longer.

Note that I didn't closely look at what is done when to see if the error handling makes sense for all cases but valgrind is happy with it and the error message is clearer now (instead 'save partial mailbox too...' it now also correctly says that it cannot reopen the folder).

  bye, Rocco
--
:wq!
diff --git a/mbox.c b/mbox.c
index 2b62f17..cad75b0 100644
--- a/mbox.c
+++ b/mbox.c
@@ -881,7 +881,7 @@ int mbox_sync_mailbox (CONTEXT *ctx, int
     dprint (1, (debugfile, "mbox_sync_mailbox: unable to reopen temp copy of 
mailbox!\n"));
     mutt_perror (tempfile);
     mutt_sleep (5);
-    return (-1);
+    goto bail;
   }
 
   if (fseeko (ctx->fp, offset, SEEK_SET) != 0 ||  /* seek the append location 
*/
@@ -939,7 +939,7 @@ int mbox_sync_mailbox (CONTEXT *ctx, int
     mutt_pretty_mailbox (savefile);
     mutt_error (_("Write failed!  Saved partial mailbox to %s"), savefile);
     mutt_sleep (5);
-    return (-1);
+    goto bail;
   }
 
   /* Restore the previous access/modification times */
@@ -954,7 +954,7 @@ int mbox_sync_mailbox (CONTEXT *ctx, int
     mutt_unblock_signals ();
     mx_fastclose_mailbox (ctx);
     mutt_error _("Fatal error!  Could not reopen mailbox!");
-    return (-1);
+    goto bail;
   }
 
   /* update the offsets of the rewritten messages */
@@ -999,7 +999,7 @@ bail:  /* Come here in case of disaster 
   FREE (&newOffset);
   FREE (&oldOffset);
 
-  if ((ctx->fp = freopen (ctx->path, "r", ctx->fp)) == NULL)
+  if (!ctx->path || (ctx->fp = freopen (ctx->path, "r", ctx->fp)) == NULL)
   {
     mutt_error _("Could not reopen mailbox!");
     mx_fastclose_mailbox (ctx);