[PATCH] Fixed some "bugs" appearing through clang-analyzer

Leo Gaspard ekleog at gmail.com
Tue Aug 21 00:36:21 CEST 2012


 * engine-gpgsm.c :
    If USE_DESCRIPTOR_PASSING is not set, some useless operations take
    place :
     * stack allocation of "int dir" ;
     * affectation of iocb_data->dir to dir.
    Fixed these by extending the #if USE_DESCRIPTOR_PASSING.

 * keylist.c :
    Same mistake twice : opd is stack-allocated and affected without
    ever being used, except in a sizeof ; where it may be replaced by
    op_data_t.

 * op-support.c :
    Useless double-assignment of err if LC_CTYPE is defined. Moved
    "err = 0" to a #else clause to avoid "err = 0; err = [...];".

 * trustlist.c :
    Same as keylist.c.

 * verify.c :
    * Removed useless assignment of tail to end at the end of the
      function parse_valid_sig ;
    * Removed useless assignment of lastp in a loop, while it won't be
      read from anywhere anylonger, in the function parse_notation.

 * vfs-mount.c :
    Same as keylist.c
---
Just wanted to say clang-analyser also reports an error in import.c line 247
(err set but not used), but adding a check for "err" makes a test fail.
I mean, adding "if (err) return err;" between lines 247 and 248 makes a test
fail.
I'm not familiar enough yet with this test framework to see where the bug comes
from, and wonder whether this non-check is a mistake or not. Anyway, there is an
error around these lines, no matter whether it is the missing check or an
useless assignment to "err".

Hoping that helps,

Leo

 src/engine-gpgsm.c | 4 +++-
 src/keylist.c      | 8 ++------
 src/op-support.c   | 3 ++-
 src/trustlist.c    | 4 +---
 src/verify.c       | 4 ----
 src/vfs-mount.c    | 6 ++----
 6 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/src/engine-gpgsm.c b/src/engine-gpgsm.c
index 0620458..1703381 100644
--- a/src/engine-gpgsm.c
+++ b/src/engine-gpgsm.c
@@ -645,7 +645,9 @@ gpgsm_set_fd (engine_gpgsm_t gpgsm, fd_type_t fd_type, const char *opt)
   char line[COMMANDLINELEN];
   char *which;
   iocb_data_t *iocb_data;
+#if USE_DESCRIPTOR_PASSING
   int dir;
+#endif
 
   switch (fd_type)
     {
@@ -668,9 +670,9 @@ gpgsm_set_fd (engine_gpgsm_t gpgsm, fd_type_t fd_type, const char *opt)
       return gpg_error (GPG_ERR_INV_VALUE);
     }
 
+#if USE_DESCRIPTOR_PASSING
   dir = iocb_data->dir;
 
-#if USE_DESCRIPTOR_PASSING
   /* We try to short-cut the communication by giving GPGSM direct
      access to the file descriptor, rather than using a pipe.  */
   iocb_data->server_fd = _gpgme_data_get_fd (iocb_data->data);
diff --git a/src/keylist.c b/src/keylist.c
index 29e30a0..2ddef91 100644
--- a/src/keylist.c
+++ b/src/keylist.c
@@ -858,7 +858,6 @@ gpgme_op_keylist_start (gpgme_ctx_t ctx, const char *pattern, int secret_only)
 {
   gpgme_error_t err;
   void *hook;
-  op_data_t opd;
 
   TRACE_BEG2 (DEBUG_CTX, "gpgme_op_keylist_start", ctx,
 	      "pattern=%s, secret_only=%i", pattern, secret_only);
@@ -871,8 +870,7 @@ gpgme_op_keylist_start (gpgme_ctx_t ctx, const char *pattern, int secret_only)
     return TRACE_ERR (err);
 
   err = _gpgme_op_data_lookup (ctx, OPDATA_KEYLIST, &hook,
-			       sizeof (*opd), release_op_data);
-  opd = hook;
+			       sizeof (* (op_data_t) 0), release_op_data);
   if (err)
     return TRACE_ERR (err);
 
@@ -898,7 +896,6 @@ gpgme_op_keylist_ext_start (gpgme_ctx_t ctx, const char *pattern[],
 {
   gpgme_error_t err;
   void *hook;
-  op_data_t opd;
 
   TRACE_BEG2 (DEBUG_CTX, "gpgme_op_keylist_ext_start", ctx,
 	      "secret_only=%i, reserved=0x%x", secret_only, reserved);
@@ -911,8 +908,7 @@ gpgme_op_keylist_ext_start (gpgme_ctx_t ctx, const char *pattern[],
     return TRACE_ERR (err);
 
   err = _gpgme_op_data_lookup (ctx, OPDATA_KEYLIST, &hook,
-			       sizeof (*opd), release_op_data);
-  opd = hook;
+			       sizeof (* (op_data_t) 0), release_op_data);
   if (err)
     return TRACE_ERR (err);
 
diff --git a/src/op-support.c b/src/op-support.c
index 000560b..aa96555 100644
--- a/src/op-support.c
+++ b/src/op-support.c
@@ -122,9 +122,10 @@ _gpgme_op_reset (gpgme_ctx_t ctx, int type)
 
   if (!reuse_engine)
     {
-      err = 0;
 #ifdef LC_CTYPE
       err = _gpgme_engine_set_locale (ctx->engine, LC_CTYPE, ctx->lc_ctype);
+#else
+      err = 0;
 #endif
 #ifdef LC_MESSAGES
       if (!err)
diff --git a/src/trustlist.c b/src/trustlist.c
index a96b6c7..30dae1f 100644
--- a/src/trustlist.c
+++ b/src/trustlist.c
@@ -172,7 +172,6 @@ gpgme_op_trustlist_start (gpgme_ctx_t ctx, const char *pattern, int max_level)
 {
   gpgme_error_t err = 0;
   void *hook;
-  op_data_t opd;
 
   TRACE_BEG2 (DEBUG_CTX, "gpgme_op_trustlist_start", ctx,
 	      "pattern=%s, max_level=%i", pattern, max_level);
@@ -185,8 +184,7 @@ gpgme_op_trustlist_start (gpgme_ctx_t ctx, const char *pattern, int max_level)
     return TRACE_ERR (err);
 
   err = _gpgme_op_data_lookup (ctx, OPDATA_TRUSTLIST, &hook,
-			       sizeof (*opd), NULL);
-  opd = hook;
+			       sizeof (* (op_data_t) 0), NULL);
   if (err)
     return TRACE_ERR (err);
 
diff --git a/src/verify.c b/src/verify.c
index a61cc95..5bb231c 100644
--- a/src/verify.c
+++ b/src/verify.c
@@ -459,7 +459,6 @@ parse_valid_sig (gpgme_signature_t sig, char *args)
 		  sig->hash_algo = strtol (end, &tail, 0);
 		  if (errno || end == tail || *tail != ' ')
 		    return gpg_error (GPG_ERR_INV_ENGINE);
-		  end = tail;
 		}
 	    }
 	}
@@ -536,10 +535,7 @@ parse_notation (gpgme_signature_t sig, gpgme_status_code_t code, char *args)
 
       /* FIXME: We could keep a pointer to the last notation in the list.  */
       while (notation && notation->next)
-	{
-	  lastp = &notation->next;
 	  notation = notation->next;
-	}
 
       if (!notation || !notation->name)
 	/* There is notation data without a previous notation
diff --git a/src/vfs-mount.c b/src/vfs-mount.c
index 327e353..4ea5ed7 100644
--- a/src/vfs-mount.c
+++ b/src/vfs-mount.c
@@ -91,7 +91,6 @@ vfs_start (gpgme_ctx_t ctx, int synchronous,
 {
   gpgme_error_t err;
   void *hook;
-  op_data_t opd;
 
   if (!command || !*command)
     return gpg_error (GPG_ERR_INV_VALUE);
@@ -102,9 +101,8 @@ vfs_start (gpgme_ctx_t ctx, int synchronous,
   if (err)
     return err;
 
-  err = _gpgme_op_data_lookup (ctx, OPDATA_VFS_MOUNT, &hook, sizeof (*opd),
-			       NULL);
-  opd = hook;
+  err = _gpgme_op_data_lookup (ctx, OPDATA_VFS_MOUNT, &hook,
+                sizeof (* (op_data_t) 0), NULL);
   if (err)
     return err;
 
-- 
1.7.11.5




More information about the Gnupg-devel mailing list