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

Re: [PATCH 16 of 16] free intermediates in error path



>>> # HG changeset patch
>>> # User Erik Hovland <erik@xxxxxxxxxxx>
>>> # Date 1236899608 25200
>>> # Branch HEAD
>>> # Node ID b195a368657041f8ad2a537dfeccf7b876b299c2
>>> # Parent  dfcfb5a6d00dec73c1ffa91e46f7fe37fe2aea50
>>> free intermediates in error path.
>>
>> Doesn't that function contain more error checks where this would leak? I
>> didn't really read the code, but it looks like it always leaks in case
>> it's not set to SmimeDefaultKey.
>
> This is where static analysis checkers shine over humans eyes. According
> to it, there are no more links in that function. And frankly, it is
> less likely to
> fail then I am in checking for all of the conditions. But I will
> attempt to figure
> out if I can develop a second opinion of that functions.

Static analyzers do shine. But only when humans use them right. Here
is the corrected patch for the same thing after fixing my usage of the
tool.

Sorry for the extra noise.

E

-- 
Erik Hovland
erik@xxxxxxxxxxx
http://hovland.org/
diff -r 83c546881839 lib.c
--- a/lib.c     Mon Mar 16 14:25:34 2009 -0700
+++ b/lib.c     Mon Mar 16 15:58:58 2009 -0700
@@ -190,6 +190,7 @@
   *p = r;
 }
 
+/* coverity[+free : arg-*0] */
 void safe_free (void *ptr)     /* __SAFE_FREE_CHECKED__ */
 {
   void **p = (void **)ptr;
diff -r 83c546881839 smime.c
--- a/smime.c   Mon Mar 16 14:25:34 2009 -0700
+++ b/smime.c   Mon Mar 16 15:58:58 2009 -0700
@@ -1387,6 +1387,7 @@
   if (!SmimeDefaultKey)
   {
     mutt_error _("Can't sign: No key specified. Use Sign As.");
+    FREE (&intermediates);
     return NULL;
   }
 
@@ -1402,6 +1403,7 @@
   if ((sfp = safe_fopen (filetosign, "w+")) == NULL)
   {
     mutt_perror (filetosign);
+    FREE (&intermediates);
     return NULL;
   }
 
@@ -1411,6 +1413,7 @@
     mutt_perror (signedfile);
     safe_fclose (&sfp);
     mutt_unlink (filetosign);
+    FREE (&intermediates);
     return NULL;
   }
   
@@ -1439,6 +1442,7 @@
     safe_fclose (&smimeout);
     mutt_unlink (signedfile);
     mutt_unlink (filetosign);
+    FREE (&intermediates);
     return NULL;
   }
   fputs (SmimePass, smimein);