From 72941db546978668aa62c82608dd83214c6f4b7e Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Wed, 27 Feb 2019 10:53:36 +0000 Subject: [PATCH] Coverity fixes 1399064 1399065 1398655 1398656 1398654 1399067 1399066 1399063 1399060 1399059 1399068 1399062 1398657 1398653 1302848 1302847 1399070 --- .gitignore | 2 ++ Makefile | 7 +++++-- config.h | 11 +++++++++++ lib/handle_pubackcomp.c | 16 +++++++--------- lib/messages_mosq.c | 3 +++ lib/property_mosq.c | 17 +++++++++++------ lib/send_mosq.c | 8 ++++---- src/conf_includedir.c | 8 ++++++-- src/context.c | 5 +---- src/database.c | 1 + src/persist.c | 7 +------ src/send_connack.c | 10 ++++------ test/Makefile | 7 ++++--- test/unit/test.c | 1 + 14 files changed, 61 insertions(+), 42 deletions(-) diff --git a/.gitignore b/.gitignore index a395a05e..c11faa12 100644 --- 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 index 9a9f5562..ab67ae0b 100644 --- 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 index 78e35a31..ad35a002 100644 --- a/config.h +++ b/config.h @@ -47,4 +47,15 @@ # endif #endif + +#ifdef __COVERITY__ +# include +/* 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 index 6c062bb4..99b2539d 100644 --- 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 index 87ad08d4..3cbcc011 100644 --- 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 index e9399fd5..e2d3488a 100644 --- 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 index 6f39b36d..a973eb61 100644 --- 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 index a23c350b..ec9f392b 100644 --- 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 index 7f999c1a..4a1b0eb8 100644 --- 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 index 193db02b..7738f07a 100644 --- a/src/database.c +++ b/src/database.c @@ -703,6 +703,7 @@ error: mosquitto__free(temp); } mosquitto_property_free_all(&properties); + UHPA_FREE(*payload, payloadlen); return rc; } diff --git a/src/persist.c b/src/persist.c index ef487879..98f79146 100644 --- 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 @@ error: 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 index 5b12bbc8..5c582e52 100644 --- 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 index aeb69f07..a25af26e 100644 --- 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 index 17a37023..d59f5104 100644 --- a/test/unit/test.c +++ b/test/unit/test.c @@ -1,3 +1,4 @@ +#include "config.h" #include #include