Coverity fixes

1399064 1399065 1398655 1398656 1398654 1399067 1399066 1399063 1399060 1399059 1399068 1399062 1398657 1398653 1302848 1302847 1399070
eclipse · Feb 27, 2019 · 72941db · 72941db
1 parent e334aad
commit 72941db
Show file tree

Hide file tree

Showing 14 changed files with 61 additions and 42 deletions.
diff --git a/.gitignore b/.gitignore
@@ -18,6 +18,8 @@ build/
 client/mosquitto_pub
 client/mosquitto_sub
 
+cov-int/
+
 dist/
 
 examples/mysql_log/mosquitto_mysql_log

diff --git a/Makefile b/Makefile
@@ -36,11 +36,14 @@ reallyclean :
  $(MAKE) -C test reallyclean
  -rm -f *.orig
 
+test : mosquitto
+ $(MAKE) -C test test
+
 ptest : mosquitto
  $(MAKE) -C test ptest
 
-test : mosquitto
- $(MAKE) -C test test
+utest : mosquitto
+ $(MAKE) -C test utest
 
 install : mosquitto
  set -e; for d in ${DIRS}; do $(MAKE) -C $${d} install; done

diff --git a/config.h b/config.h
@@ -47,4 +47,15 @@
 # endif
 #endif
 
+
+#ifdef __COVERITY__
+# include <stdint.h>
+/* These are "wrong", but we don't use them so it doesn't matter */
+# define _Float32 uint32_t
+# define _Float32x uint32_t
+# define _Float64 uint64_t
+# define _Float64x uint64_t
+# define _Float128 uint64_t
+#endif
+
 #endif
diff --git a/lib/handle_pubackcomp.c b/lib/handle_pubackcomp.c
@@ -70,14 +70,12 @@ int handle__pubackcomp(struct mosquitto *mosq, const char *type)
  /* Immediately free, we don't do anything with Reason String or User Property at the moment */
  mosquitto_property_free_all(&properties);
 
- if(mid){
- rc = db__message_delete(db, mosq, mid, mosq_md_out, mosq_ms_wait_for_pubcomp, qos);
- if(rc == MOSQ_ERR_NOT_FOUND){
- log__printf(mosq, MOSQ_LOG_WARNING, "Warning: Received %s from %s for an unknown packet identifier %d.", type, mosq->id, mid);
- return MOSQ_ERR_SUCCESS;
- }else{
- return rc;
- }
+ rc = db__message_delete(db, mosq, mid, mosq_md_out, mosq_ms_wait_for_pubcomp, qos);
+ if(rc == MOSQ_ERR_NOT_FOUND){
+ log__printf(mosq, MOSQ_LOG_WARNING, "Warning: Received %s from %s for an unknown packet identifier %d.", type, mosq->id, mid);
+ return MOSQ_ERR_SUCCESS;
+ }else{
+ return rc;
  }
 #else
  log__printf(mosq, MOSQ_LOG_DEBUG, "Client %s received %s (Mid: %d)", mosq->id, type, mid);
@@ -101,8 +99,8 @@ int handle__pubackcomp(struct mosquitto *mosq, const char *type)
  pthread_mutex_unlock(&mosq->callback_mutex);
  mosquitto_property_free_all(&properties);
  }
-#endif
 
  return MOSQ_ERR_SUCCESS;
+#endif
 }
 
diff --git a/lib/messages_mosq.c b/lib/messages_mosq.c
@@ -230,6 +230,7 @@ int message__remove(struct mosquitto *mosq, uint16_t mid, enum mosquitto_msg_dir
  while(cur){
  if(cur->msg.mid == mid){
  if(cur->msg.qos != qos){
+ pthread_mutex_unlock(&mosq->out_message_mutex);
  return MOSQ_ERR_PROTOCOL;
  }
  if(prev){
@@ -287,6 +288,7 @@ int message__remove(struct mosquitto *mosq, uint16_t mid, enum mosquitto_msg_dir
  while(cur){
  if(cur->msg.mid == mid){
  if(cur->msg.qos != qos){
+ pthread_mutex_unlock(&mosq->in_message_mutex);
  return MOSQ_ERR_PROTOCOL;
  }
  if(prev){
@@ -382,6 +384,7 @@ int message__out_update(struct mosquitto *mosq, uint16_t mid, enum mosquitto_msg
  while(message){
  if(message->msg.mid == mid){
  if(message->msg.qos != qos){
+ pthread_mutex_unlock(&mosq->out_message_mutex);
  return MOSQ_ERR_PROTOCOL;
  }
  message->state = state;

diff --git a/lib/property_mosq.c b/lib/property_mosq.c
@@ -39,6 +39,8 @@ int property__read(struct mosquitto__packet *packet, int32_t *len, mosquitto_pro
  char *str1, *str2;
  int slen1, slen2;
 
+ if(!property) return MOSQ_ERR_INVAL;
+
  rc = packet__read_varint(packet, &property_identifier, NULL);
  if(rc) return rc;
  *len -= 1;
@@ -131,7 +133,7 @@ int property__read(struct mosquitto__packet *packet, int32_t *len, mosquitto_pro
  break;
 
  default:
- log__printf(NULL, MOSQ_LOG_DEBUG, "Unsupported property type: %d", byte);
+ log__printf(NULL, MOSQ_LOG_DEBUG, "Unsupported property type: %d", property_identifier);
  return MOSQ_ERR_MALFORMED_PACKET;
  }
 
@@ -154,6 +156,10 @@ int property__read_all(int command, struct mosquitto__packet *packet, mosquitto_
  * same order for all */
  while(proplen > 0){
  p = mosquitto__calloc(1, sizeof(mosquitto_property));
+ if(!p){
+ mosquitto_property_free_all(properties);
+ return MOSQ_ERR_NOMEM;
+ }
 
  rc = property__read(packet, &proplen, p); 
  if(rc){
@@ -1002,7 +1008,7 @@ const mosquitto_property *mosquitto_property_read_binary(const mosquitto_propert
  if(value){
  *len = p->value.bin.len;
  *value = malloc(*len);
- if(!value) return NULL;
+ if(!(*value)) return NULL;
 
  memcpy(*value, p->value.bin.v, *len);
  }
@@ -1031,12 +1037,11 @@ const mosquitto_property *mosquitto_property_read_string(const mosquitto_propert
 
  if(value){
  *value = calloc(1, p->value.s.len+1);
- if(!value) return NULL;
+ if(!(*value)) return NULL;
 
  memcpy(*value, p->value.s.v, p->value.s.len);
  }
 
-
  return p;
 }
 
@@ -1052,13 +1057,13 @@ const mosquitto_property *mosquitto_property_read_string_pair(const mosquitto_pr
 
  if(name){
  *name = calloc(1, p->name.len+1);
- if(!name) return NULL;
+ if(!(*name)) return NULL;
  memcpy(*name, p->name.v, p->name.len);
  }
 
  if(value){
  *value = calloc(1, p->value.s.len+1);
- if(!value){
+ if(!(*value)){
  if(name) free(*name);
  return NULL;
  }

diff --git a/lib/send_mosq.c b/lib/send_mosq.c
@@ -68,9 +68,9 @@ int send__pingresp(struct mosquitto *mosq)
 int send__puback(struct mosquitto *mosq, uint16_t mid)
 {
 #ifdef WITH_BROKER
- if(mosq) log__printf(NULL, MOSQ_LOG_DEBUG, "Sending PUBACK to %s (Mid: %d)", mosq->id, mid);
+ log__printf(NULL, MOSQ_LOG_DEBUG, "Sending PUBACK to %s (Mid: %d)", mosq->id, mid);
 #else
- if(mosq) log__printf(mosq, MOSQ_LOG_DEBUG, "Client %s sending PUBACK (Mid: %d)", mosq->id, mid);
+ log__printf(mosq, MOSQ_LOG_DEBUG, "Client %s sending PUBACK (Mid: %d)", mosq->id, mid);
 #endif
  util__increment_receive_quota(mosq);
  /* We don't use Reason String or User Property yet. */
@@ -80,9 +80,9 @@ int send__puback(struct mosquitto *mosq, uint16_t mid)
 int send__pubcomp(struct mosquitto *mosq, uint16_t mid)
 {
 #ifdef WITH_BROKER
- if(mosq) log__printf(NULL, MOSQ_LOG_DEBUG, "Sending PUBCOMP to %s (Mid: %d)", mosq->id, mid);
+ log__printf(NULL, MOSQ_LOG_DEBUG, "Sending PUBCOMP to %s (Mid: %d)", mosq->id, mid);
 #else
- if(mosq) log__printf(mosq, MOSQ_LOG_DEBUG, "Client %s sending PUBCOMP (Mid: %d)", mosq->id, mid);
+ log__printf(mosq, MOSQ_LOG_DEBUG, "Client %s sending PUBCOMP (Mid: %d)", mosq->id, mid);
 #endif
  util__increment_receive_quota(mosq);
  /* We don't use Reason String or User Property yet. */

diff --git a/src/conf_includedir.c b/src/conf_includedir.c
@@ -102,7 +102,9 @@ int config__get_dir_files(const char *include_dir, char ***files, int *file_coun
 
  FindClose(fh);
 
- qsort(l_files, l_file_count, sizeof(char *), scmp_p);
+ if(l_files){
+ qsort(l_files, l_file_count, sizeof(char *), scmp_p);
+ }
  *files = l_files;
  *file_count = l_file_count;
 
@@ -166,7 +168,9 @@ int config__get_dir_files(const char *include_dir, char ***files, int *file_coun
  }
  closedir(dh);
 
- qsort(l_files, l_file_count, sizeof(char *), scmp_p);
+ if(l_files){
+ qsort(l_files, l_file_count, sizeof(char *), scmp_p);
+ }
  *files = l_files;
  *file_count = l_file_count;
 

diff --git a/src/context.c b/src/context.c
@@ -152,7 +152,7 @@ void context__cleanup(struct mosquitto_db *db, struct mosquitto *context, bool d
  context->password = NULL;
 
  net__socket_close(db, context);
- if((do_free || context->clean_start) && db){
+ if(do_free || context->clean_start){
  sub__clean_session(db, context);
  db__messages_delete(db, context);
  }
@@ -163,9 +163,6 @@ void context__cleanup(struct mosquitto_db *db, struct mosquitto *context, bool d
  context__send_will(db, context);
 
  if(context->id){
- assert(db); /* db can only be NULL here if the client hasn't sent a
- CONNECT and hence wouldn't have an id. */
-
  context__remove_from_by_id(db, context);
  mosquitto__free(context->id);
  context->id = NULL;

diff --git a/src/database.c b/src/database.c
@@ -703,6 +703,7 @@ int db__message_store(struct mosquitto_db *db, const struct mosquitto *source, u
  mosquitto__free(temp);
  }
  mosquitto_property_free_all(&properties);
+ UHPA_FREE(*payload, payloadlen);
  return rc;
 }
 

diff --git a/src/persist.c b/src/persist.c
@@ -170,7 +170,7 @@ static int persist__message_store_write(struct mosquitto_db *db, FILE *db_fptr)
  }else{
  tlen = 0;
  }
- length = sizeof(dbid_t) + 2+strlen(stored->source_id) +
+ length = sizeof(dbid_t) + sizeof(uint16_t) +
  sizeof(uint16_t) + sizeof(uint16_t) +
  2+tlen + sizeof(uint32_t) +
  stored->payloadlen + sizeof(uint8_t) + sizeof(uint8_t)
@@ -770,7 +770,6 @@ static int persist__msg_store_chunk_restore(struct mosquitto_db *db, FILE *db_fp
  mosquitto__free(load);
  fclose(db_fptr);
  mosquitto__free(source.id);
- mosquitto__free(source.id);
  return rc;
  }
 
@@ -808,8 +807,6 @@ static int persist__msg_store_chunk_restore(struct mosquitto_db *db, FILE *db_fp
  }else{
  mosquitto__free(load);
  fclose(db_fptr);
- mosquitto__free(topic);
- UHPA_FREE(payload, payloadlen);
  return rc;
  }
 error:
@@ -818,8 +815,6 @@ static int persist__msg_store_chunk_restore(struct mosquitto_db *db, FILE *db_fp
  fclose(db_fptr);
  mosquitto__free(source.id);
  mosquitto__free(source.username);
- mosquitto__free(topic);
- UHPA_FREE(payload, payloadlen);
  return 1;
 }
 

diff --git a/src/send_connack.c b/src/send_connack.c
@@ -36,12 +36,10 @@ int send__connack(struct mosquitto_db *db, struct mosquitto *context, int ack, i
  return rc;
  }
 
- if(context){
- if(context->id){
- log__printf(NULL, MOSQ_LOG_DEBUG, "Sending CONNACK to %s (%d, %d)", context->id, ack, reason_code);
- }else{
- log__printf(NULL, MOSQ_LOG_DEBUG, "Sending CONNACK to %s (%d, %d)", context->address, ack, reason_code);
- }
+ if(context->id){
+ log__printf(NULL, MOSQ_LOG_DEBUG, "Sending CONNACK to %s (%d, %d)", context->id, ack, reason_code);
+ }else{
+ log__printf(NULL, MOSQ_LOG_DEBUG, "Sending CONNACK to %s (%d, %d)", context->address, ack, reason_code);
  }
 
  remaining_length = 2;

diff --git a/test/Makefile b/test/Makefile
@@ -4,14 +4,15 @@ include ../config.mk
 
 all :
 
-test :
+test : utest
  $(MAKE) -C broker test
  $(MAKE) -C lib test
- $(MAKE) -C unit test
 
-ptest :
+ptest : utest
  $(MAKE) -C broker ptest
  $(MAKE) -C lib ptest
+
+utest :
  $(MAKE) -C unit test
 
 clean : 

diff --git a/test/unit/test.c b/test/unit/test.c
@@ -1,3 +1,4 @@
+#include "config.h"
 #include <stdio.h>
 
 #include <CUnit/CUnit.h>