Fix Coverity scan issues.

1302840 1353078 1431131 1431134 1431135 1436826 1436827 1436828 1436830 1436831 1436834 1436835 1436838 1436839 1436840 1436844 1436846 1436853 1436854 1436859 1436860 1436861 1436863 1436867
eclipse · Nov 23, 2020 · d26534e · d26534e
1 parent d1bf2c0
commit d26534e
Show file tree

Hide file tree

Showing 14 changed files with 54 additions and 39 deletions.
diff --git a/client/sub_client_output.c b/client/sub_client_output.c
@@ -152,13 +152,13 @@ static void write_json_payload(const char *payload, int payloadlen)
 static int json_print_properties(cJSON *root, const mosquitto_property *properties)
 {
  int identifier;
- uint8_t i8value;
- uint16_t i16value;
- uint32_t i32value;
- char *strname, *strvalue;
- char *binvalue;
+ uint8_t i8value = 0;
+ uint16_t i16value = 0;
+ uint32_t i32value = 0;
+ char *strname = NULL, *strvalue = NULL;
+ char *binvalue = NULL;
  cJSON *tmp, *prop_json, *user_json = NULL;
- const mosquitto_property *prop;
+ const mosquitto_property *prop = NULL;
 
  prop_json = cJSON_CreateObject();
  if(prop_json == NULL){
@@ -187,13 +187,15 @@ static int json_print_properties(cJSON *root, const mosquitto_property *properti
  if(strvalue == NULL) return MOSQ_ERR_NOMEM;
  tmp = cJSON_CreateString(strvalue);
  free(strvalue);
+ strvalue = NULL;
  break;
 
  case MQTT_PROP_CORRELATION_DATA:
  mosquitto_property_read_binary(prop, MQTT_PROP_CORRELATION_DATA, (void **)&binvalue, &i16value, false);
  if(binvalue == NULL) return MOSQ_ERR_NOMEM;
  tmp = cJSON_CreateString(binvalue);
  free(binvalue);
+ binvalue = NULL;
  break;
 
  case MQTT_PROP_SUBSCRIPTION_IDENTIFIER:

diff --git a/lib/connect.c b/lib/connect.c
@@ -155,7 +155,7 @@ static int mosquitto__reconnect(struct mosquitto *mosq, bool blocking)
  int rc;
 
  if(!mosq) return MOSQ_ERR_INVAL;
- if(!mosq->host || mosq->port < 0) return MOSQ_ERR_INVAL;
+ if(!mosq->host) return MOSQ_ERR_INVAL;
 
  if(mosq->connect_properties){
  if(mosq->protocol != mosq_p_mqtt5) return MOSQ_ERR_NOT_SUPPORTED;

diff --git a/lib/handle_suback.c b/lib/handle_suback.c
@@ -71,6 +71,9 @@ int handle__suback(struct mosquitto *mosq)
  rc = packet__read_byte(&mosq->in_packet, &qos);
  if(rc){
  mosquitto__free(granted_qos);
+#ifdef WITH_BROKER
+ mosquitto_property_free_all(&properties);
+#endif
  return rc;
  }
  granted_qos[i] = (int)qos;

diff --git a/lib/send_connect.c b/lib/send_connect.c
@@ -115,6 +115,7 @@ int send__connect(struct mosquitto *mosq, uint16_t keepalive, bool clean_session
  * username before checking password. */
  if(mosq->protocol == mosq_p_mqtt31 || mosq->protocol == mosq_p_mqtt311){
  if(password != NULL && username == NULL){
+ mosquitto__free(packet);
  return MOSQ_ERR_INVAL;
  }
  }

diff --git a/src/bridge.c b/src/bridge.c
@@ -323,7 +323,7 @@ int bridge__connect(struct mosquitto *context)
 {
  int rc, rc2;
  int i;
- char *notification_topic;
+ char *notification_topic = NULL;
  size_t notification_topic_len;
  uint8_t notification_payload;
 
@@ -417,6 +417,7 @@ int bridge__connect(struct mosquitto *context)
  if(rc > 0){
  if(rc == MOSQ_ERR_TLS){
  net__socket_close(context);
+ mosquitto__free(notification_topic);
  return rc; /* Error already printed */
  }else if(rc == MOSQ_ERR_ERRNO){
  log__printf(NULL, MOSQ_LOG_ERR, "Error creating bridge: %s.", strerror(errno));
@@ -735,9 +736,9 @@ void bridge_check(void)
  }else if(rc == 0){
  rc = bridge__connect_step2(context);
  if(rc == MOSQ_ERR_SUCCESS){
- rc = mux__add_in(context);
+ mux__add_in(context);
  if(context->current_out_packet){
- rc = mux__add_out(context);
+ mux__add_out(context);
  }
  }else if(rc == MOSQ_ERR_CONN_PENDING){
  context->bridge->restart_t = 0;
@@ -779,7 +780,7 @@ void bridge_check(void)
  }
  mux__add_in(context);
  if(context->current_out_packet){
- rc = mux__add_out(context);
+ mux__add_out(context);
  }
  }else{
  context->bridge->cur_address++;

diff --git a/src/bridge_topic.c b/src/bridge_topic.c
@@ -106,7 +106,7 @@ int bridge__add_topic(struct mosquitto__bridge *bridge, const char *topic, enum
  if(direction != bd_out && direction != bd_in && direction != bd_both){
  return MOSQ_ERR_INVAL;
  }
- if(qos < 0 || qos > 2){
+ if(qos > 2){
  return MOSQ_ERR_INVAL;
  }
  if(local_prefix && mosquitto_pub_topic_check(local_prefix)){

diff --git a/src/conf.c b/src/conf.c
@@ -2049,7 +2049,7 @@ int config__read_file_core(struct mosquitto__config *config, bool reload, struct
  strtok_r(NULL, "", &saveptr);
  }
  qos = (uint8_t)atoi(token);
- if(qos < 0 || qos > 2){
+ if(qos > 2){
  log__printf(NULL, MOSQ_LOG_ERR, "Error: Invalid bridge QoS level '%s'.", token);
  return MOSQ_ERR_INVAL;
  }

diff --git a/src/control.c b/src/control.c
@@ -63,9 +63,9 @@ int control__process(struct mosquitto *context, struct mosquitto_msg_store *stor
  }
 
  if(stored->qos == 1){
- if(send__puback(context, stored->source_mid, event_data.reason_code, properties)) rc = 1;
+ if(send__puback(context, stored->source_mid, MQTT_RC_SUCCESS, properties)) rc = 1;
  }else if(stored->qos == 2){
- if(send__pubrec(context, stored->source_mid, event_data.reason_code, properties)) rc = 1;
+ if(send__pubrec(context, stored->source_mid, MQTT_RC_SUCCESS, properties)) rc = 1;
  }
  mosquitto_property_free_all(&properties);
 

diff --git a/src/handle_unsubscribe.c b/src/handle_unsubscribe.c
@@ -118,6 +118,7 @@ int handle__unsubscribe(struct mosquitto *context)
  break;
  default:
  mosquitto__free(sub);
+ mosquitto__free(reason_codes);
  return rc;
  }
 

diff --git a/src/net.c b/src/net.c
@@ -638,6 +638,7 @@ static int net__socket_listen_tcp(struct mosquitto__listener *listener)
  if(!listener->socks){
  log__printf(NULL, MOSQ_LOG_ERR, "Error: Out of memory.");
  freeaddrinfo(ainfo);
+ COMPAT_CLOSE(sock);
  return MOSQ_ERR_NOMEM;
  }
  listener->socks[listener->sock_count-1] = sock;
@@ -726,6 +727,7 @@ static int net__socket_listen_unix(struct mosquitto__listener *listener)
  listener->socks = mosquitto__realloc(listener->socks, sizeof(mosq_sock_t)*(size_t)listener->sock_count);
  if(!listener->socks){
  log__printf(NULL, MOSQ_LOG_ERR, "Error: Out of memory.");
+ COMPAT_CLOSE(sock);
  return MOSQ_ERR_NOMEM;
  }
  listener->socks[listener->sock_count-1] = sock;

diff --git a/src/persist_read.c b/src/persist_read.c
@@ -78,7 +78,6 @@ int persist__read_string_len(FILE *db_fptr, char **str, uint16_t len)
  if(len){
  s = mosquitto__malloc(len+1U);
  if(!s){
- fclose(db_fptr);
  log__printf(NULL, MOSQ_LOG_ERR, "Error: Out of memory.");
  return MOSQ_ERR_NOMEM;
  }
@@ -186,7 +185,6 @@ static int persist__client_chunk_restore(FILE *db_fptr)
  rc = persist__chunk_client_read_v234(db_fptr, &chunk, db_version);
  }
  if(rc > 0){
- fclose(db_fptr);
  return rc;
  }else if(rc < 0){
  /* Client not loaded, but otherwise not an error */
@@ -238,7 +236,6 @@ static int persist__client_msg_chunk_restore(FILE *db_fptr, uint32_t length)
  rc = persist__chunk_client_msg_read_v234(db_fptr, &chunk);
  }
  if(rc){
- fclose(db_fptr);
  return rc;
  }
 
@@ -267,7 +264,6 @@ static int persist__msg_store_chunk_restore(FILE *db_fptr, uint32_t length)
  rc = persist__chunk_msg_store_read_v234(db_fptr, &chunk, db_version);
  }
  if(rc){
- fclose(db_fptr);
  return rc;
  }
 
@@ -281,7 +277,6 @@ static int persist__msg_store_chunk_restore(FILE *db_fptr, uint32_t length)
  }
  load = mosquitto__calloc(1, sizeof(struct mosquitto_msg_store_load));
  if(!load){
- fclose(db_fptr);
  mosquitto__free(chunk.source.id);
  mosquitto__free(chunk.source.username);
  mosquitto__free(chunk.topic);
@@ -309,7 +304,7 @@ static int persist__msg_store_chunk_restore(FILE *db_fptr, uint32_t length)
 
  stored = mosquitto__calloc(1, sizeof(struct mosquitto_msg_store));
  if(stored == NULL){
- fclose(db_fptr);
+ mosquitto__free(load);
  mosquitto__free(chunk.source.id);
  mosquitto__free(chunk.source.username);
  mosquitto__free(chunk.topic);
@@ -343,7 +338,6 @@ static int persist__msg_store_chunk_restore(FILE *db_fptr, uint32_t length)
  return MOSQ_ERR_SUCCESS;
  }else{
  mosquitto__free(load);
- fclose(db_fptr);
  return rc;
  }
 }
@@ -364,7 +358,6 @@ static int persist__retain_chunk_restore(FILE *db_fptr)
  rc = persist__chunk_retain_read_v234(db_fptr, &chunk);
  }
  if(rc){
- fclose(db_fptr);
  return rc;
  }
 
@@ -393,7 +386,6 @@ static int persist__sub_chunk_restore(FILE *db_fptr)
  rc = persist__chunk_sub_read_v234(db_fptr, &chunk);
  }
  if(rc){
- fclose(db_fptr);
  return rc;
  }
 
@@ -494,23 +486,38 @@ int persist__restore(void)
  break;
 
  case DB_CHUNK_MSG_STORE:
- if(persist__msg_store_chunk_restore(fptr, length)) return 1;
+ if(persist__msg_store_chunk_restore(fptr, length)){
+ fclose(fptr);
+ return 1;
+ }
  break;
 
  case DB_CHUNK_CLIENT_MSG:
- if(persist__client_msg_chunk_restore(fptr, length)) return 1;
+ if(persist__client_msg_chunk_restore(fptr, length)){
+ fclose(fptr);
+ return 1;
+ }
  break;
 
  case DB_CHUNK_RETAIN:
- if(persist__retain_chunk_restore(fptr)) return 1;
+ if(persist__retain_chunk_restore(fptr)){
+ fclose(fptr);
+ return 1;
+ }
  break;
 
  case DB_CHUNK_SUB:
- if(persist__sub_chunk_restore(fptr)) return 1;
+ if(persist__sub_chunk_restore(fptr)){
+ fclose(fptr);
+ return 1;
+ }
  break;
 
  case DB_CHUNK_CLIENT:
- if(persist__client_chunk_restore(fptr)) return 1;
+ if(persist__client_chunk_restore(fptr)){
+ fclose(fptr);
+ return 1;
+ }
  break;
 
  default:
@@ -519,7 +526,6 @@ int persist__restore(void)
  break;
  }
  }
- if(rlen < 0) goto error;
  }else{
  log__printf(NULL, MOSQ_LOG_ERR, "Error: Unable to restore persistent database. Unrecognised file format.");
  rc = 1;

diff --git a/src/send_connack.c b/src/send_connack.c
@@ -71,12 +71,14 @@ int send__connack(struct mosquitto *context, uint8_t ack, uint8_t reason_code, c
 
  if(packet__check_oversize(context, remaining_length)){
  mosquitto_property_free_all(&connack_props);
- mosquitto__free(packet);
  return MOSQ_ERR_OVERSIZE_PACKET;
  }
 
  packet = mosquitto__calloc(1, sizeof(struct mosquitto__packet));
- if(!packet) return MOSQ_ERR_NOMEM;
+ if(!packet){
+ mosquitto_property_free_all(&connack_props);
+ return MOSQ_ERR_NOMEM;
+ }
 
  packet->command = CMD_CONNACK;
  packet->remaining_length = remaining_length;

diff --git a/src/subs.c b/src/subs.c
@@ -252,6 +252,8 @@ static int sub__add_shared(struct mosquitto *context, uint8_t qos, uint32_t iden
  if(i == context->shared_sub_count){
  shared_subs = mosquitto__realloc(context->shared_subs, sizeof(struct mosquitto__subhier_ref *)*(size_t)(context->shared_sub_count + 1));
  if(!shared_subs){
+ mosquitto__free(shared_ref);
+ context->shared_subs[context->shared_sub_count-1] = NULL;
  sub__remove_shared_leaf(subhier, shared, newleaf);
  return MOSQ_ERR_NOMEM;
  }
@@ -579,6 +581,8 @@ int sub__add(struct mosquitto *context, const char *sub, uint8_t qos, uint32_t i
 
  topiclen = strlen(topics[0]);
  if(topiclen > UINT16_MAX){
+ mosquitto__free(local_sub);
+ mosquitto__free(topics);
  return MOSQ_ERR_INVAL;
  }
  HASH_FIND(hh, *root, topics[0], topiclen, subhier);

diff --git a/src/sys_tree.c b/src/sys_tree.c
@@ -85,13 +85,6 @@ static void sys_tree__update_clients(char *buf)
 
  if(disconnected_count != count_total-count_by_sock){
  disconnected_count = count_total-count_by_sock;
- if(disconnected_count < 0){
- /* If a client has connected but not sent a CONNECT at this point,
- * then it is possible that count_by_sock will be bigger than
- * count_total, causing a negative number. This situation should
- * not last for long, so just cap at zero and ignore. */
- disconnected_count = 0;
- }
  len = (uint32_t)snprintf(buf, BUFLEN, "%d", disconnected_count);
  db__messages_easy_queue(NULL, "$SYS/broker/clients/inactive", SYS_TREE_QOS, len, buf, 1, 60, NULL);
  db__messages_easy_queue(NULL, "$SYS/broker/clients/disconnected", SYS_TREE_QOS, len, buf, 1, 60, NULL);