From c7cafa94c41a86c427c6cd8825c36b0b1f3fa7f7 Mon Sep 17 00:00:00 2001 From: Tobias Reisinger Date: Sat, 14 Nov 2020 00:34:20 +0100 Subject: [PATCH] Fix more clang-tidy warnings --- .clang-tidy | 2 +- include/database.h | 4 +- include/models/controller.h | 8 +- include/models/macro.h | 3 - include/models/schedule.h | 3 - src/cache.c | 8 +- src/cli.c | 1 - src/database.c | 52 ++-- src/endpoints/api_v1_controllers_STR.c | 1 - src/endpoints/api_v1_controllers_discover.c | 254 ++++++++++---------- src/endpoints/api_v1_macros_STR.c | 1 - src/endpoints/api_v1_schedules_STR.c | 1 - src/endpoints/api_v1_tags.c | 1 - src/handlers/http.c | 8 +- src/handlers/mqtt.c | 2 +- src/helpers/connect_server.c | 7 +- src/models/controller.c | 8 +- src/models/macro.c | 8 +- src/models/macro_action.c | 8 +- src/models/period.c | 5 +- src/router.c | 1 - 21 files changed, 178 insertions(+), 208 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 154372e..1e0f5e6 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,2 +1,2 @@ --- -Checks: 'clang-diagnostics-*,clang-analyzer-*,linuxkernel-*,modernize-*,readability-*' +Checks: 'clang-diagnostics-*,clang-analyzer-*,linuxkernel-*,modernize-*,readability-*,-readability-magic-numbers,portability-*' diff --git a/include/database.h b/include/database.h index c393013..a97879c 100644 --- a/include/database.h +++ b/include/database.h @@ -21,10 +21,10 @@ void database_transaction_begin(database_transaction_lock *lock); void -database_transaction_commit(database_transaction_lock *lock); +database_transaction_commit(const database_transaction_lock *lock); void -database_transaction_rollback(database_transaction_lock *lock); +database_transaction_rollback(const database_transaction_lock *lock); int diff --git a/include/models/controller.h b/include/models/controller.h index e144a0d..f12f96a 100644 --- a/include/models/controller.h +++ b/include/models/controller.h @@ -25,16 +25,16 @@ typedef struct } controller_t; void -controller_free(controller_t* contoller); +controller_free(controller_t *controller); int -controller_save(controller_t* contoller); +controller_save(controller_t *controller); int -controller_remove(controller_t* contoller); +controller_remove(controller_t *controller); cJSON* -controller_to_json(controller_t* contoller); +controller_to_json(controller_t *controller); controller_t* controller_get_by_id(int id); diff --git a/include/models/macro.h b/include/models/macro.h index d532902..29b3e70 100644 --- a/include/models/macro.h +++ b/include/models/macro.h @@ -30,9 +30,6 @@ macro_free_list(macro_t **macro); cJSON* macro_to_json(macro_t *macro); -void -macro_free_list(macro_t **macros_list); - macro_t* macro_get_by_id(int id); diff --git a/include/models/schedule.h b/include/models/schedule.h index 21ee3e9..c879eec 100644 --- a/include/models/schedule.h +++ b/include/models/schedule.h @@ -35,9 +35,6 @@ schedule_free_list(schedule_t **schedule); cJSON* schedule_to_json(schedule_t *schedule); -void -schedule_free_list(schedule_t **schedules_list); - uint16_t* schedule_periods_to_blob(schedule_t *schedule); diff --git a/src/cache.c b/src/cache.c index 00e0ec6..37516f0 100644 --- a/src/cache.c +++ b/src/cache.c @@ -42,11 +42,9 @@ cache_get_value(char *key) { break; } - else - { - LOGGER_WARNING("failed selecting %s from cache: %s\n", key, sqlite3_errstr(s)); - break; - } + + LOGGER_WARNING("failed selecting %s from cache: %s\n", key, sqlite3_errstr(s)); + break; } } diff --git a/src/cli.c b/src/cli.c index 3d0b003..ac96c1c 100644 --- a/src/cli.c +++ b/src/cli.c @@ -47,5 +47,4 @@ cli_parse(int argc, const char **argv, config_t *config) printf("%s\n", EMGAUWA_CORE_VERSION); exit(0); } - return; } diff --git a/src/database.c b/src/database.c index 13f07a3..76cb9f2 100644 --- a/src/database.c +++ b/src/database.c @@ -90,8 +90,6 @@ database_migrate() default: break; } - - return; } void @@ -106,7 +104,7 @@ database_transaction_begin(database_transaction_lock *lock) } void -database_transaction_commit(database_transaction_lock *lock) +database_transaction_commit(const database_transaction_lock *lock) { if(transaction_lock == lock) { @@ -117,7 +115,7 @@ database_transaction_commit(database_transaction_lock *lock) } void -database_transaction_rollback(database_transaction_lock *lock) +database_transaction_rollback(const database_transaction_lock *lock) { if(transaction_lock == lock) { @@ -147,12 +145,10 @@ database_helper_get_id(sqlite3_stmt *stmt) { break; } - else - { - LOGGER_ERR("error selecting id from database: %s\n", sqlite3_errstr(s)); - sqlite3_finalize(stmt); - return 0; - } + + LOGGER_ERR("error selecting id from database: %s\n", sqlite3_errstr(s)); + sqlite3_finalize(stmt); + return 0; } } @@ -191,16 +187,14 @@ database_helper_get_ids(sqlite3_stmt *stmt) { break; } - else + + if(result) { - if(result) - { - free(result); - } - LOGGER_ERR("error selecting ids from database: %s\n", sqlite3_errstr(s)); - sqlite3_finalize(stmt); - return NULL; + free(result); } + LOGGER_ERR("error selecting ids from database: %s\n", sqlite3_errstr(s)); + sqlite3_finalize(stmt); + return NULL; } } @@ -238,16 +232,14 @@ database_helper_get_string(sqlite3_stmt *stmt) { break; } - else + + if(result) { - if(result) - { - free(result); - } - LOGGER_ERR("error selecting string from database: %s\n", sqlite3_errstr(s)); - sqlite3_finalize(stmt); - return NULL; + free(result); } + LOGGER_ERR("error selecting string from database: %s\n", sqlite3_errstr(s)); + sqlite3_finalize(stmt); + return NULL; } } @@ -284,11 +276,9 @@ database_helper_get_strings(sqlite3_stmt *stmt) { break; } - else - { - LOGGER_ERR("error selecting strings from database: %s\n", sqlite3_errstr(s)); - break; - } + + LOGGER_ERR("error selecting strings from database: %s\n", sqlite3_errstr(s)); + break; } } sqlite3_finalize(stmt); diff --git a/src/endpoints/api_v1_controllers_STR.c b/src/endpoints/api_v1_controllers_STR.c index 25d375f..5d5c370 100644 --- a/src/endpoints/api_v1_controllers_STR.c +++ b/src/endpoints/api_v1_controllers_STR.c @@ -168,5 +168,4 @@ api_v1_controllers_STR_DELETE(struct mg_connection *nc, struct http_message *hm, M_RESPONSE_MSG(LOGGER_DEBUG, response, 200, "deleted controller"); } controller_free(controller); - return; } diff --git a/src/endpoints/api_v1_controllers_discover.c b/src/endpoints/api_v1_controllers_discover.c index 37bae48..d3ad5cd 100644 --- a/src/endpoints/api_v1_controllers_discover.c +++ b/src/endpoints/api_v1_controllers_discover.c @@ -21,7 +21,8 @@ typedef enum static int bind_tcp_server(const char *addr, const char *port, int max_client_backlog) { - struct addrinfo hints, *res; + struct addrinfo hints; + struct addrinfo *res; int fd; int status; @@ -132,7 +133,8 @@ api_v1_controllers_discover_PUT(struct mg_connection *nc, struct http_message *h struct sockaddr_storage their_addr; socklen_t addr_size; - int client_fd, s_ret; + int s_ret; + int client_fd; fd_set accept_fds; struct timeval timeout; @@ -155,143 +157,141 @@ api_v1_controllers_discover_PUT(struct mg_connection *nc, struct http_message *h { break; } - else + + if((client_fd = accept(discover_server_socket, (struct sockaddr *) &their_addr, &addr_size)) < 0) { - if((client_fd = accept(discover_server_socket, (struct sockaddr *) &their_addr, &addr_size)) < 0) + LOGGER_ERR("error accepting client %s\n", strerror(errno)); + continue; + } + + size_t payload_length; + + if(recv(client_fd, &payload_length, sizeof(payload_length), 0) <= 0) + { + LOGGER_ERR("error receiving header from client\n"); + continue; + } + + char *answer_payload = (char*)malloc((payload_length)); + if(recv(client_fd, answer_payload, payload_length, 0) <= 0) + { + LOGGER_ERR("error receiving payload from client\n"); + continue; + } + + struct sockaddr_in addr; + socklen_t client_addr_size = sizeof(struct sockaddr_in); + if(getpeername(client_fd, (struct sockaddr *)&addr, &client_addr_size) != 0) + { + + LOGGER_ERR("error receiving payload from client\n"); + continue; + } + + LOGGER_DEBUG("received info for discovered controller\n"); + + uuid_t discovered_id; + + mpack_tree_t tree; + mpack_tree_init_data(&tree, answer_payload, payload_length); + mpack_tree_parse(&tree); + mpack_node_t root = mpack_tree_root(&tree); + + memcpy(discovered_id, mpack_node_data(mpack_node_map_uint(root, DISCOVERY_MAPPING_ID)), sizeof(uuid_t)); + + uint16_t discovered_command_port = mpack_node_u16(mpack_node_map_uint(root, DISCOVERY_MAPPING_COMMAND_PORT)); + uint8_t discovered_relay_count = mpack_node_u8(mpack_node_map_uint(root, DISCOVERY_MAPPING_RELAY_COUNT)); + const char *discovered_name = mpack_node_str(mpack_node_map_uint(root, DISCOVERY_MAPPING_NAME)); + size_t discovered_name_len = mpack_node_strlen(mpack_node_map_uint(root, DISCOVERY_MAPPING_NAME)); + + if(discovered_name_len > MAX_NAME_LENGTH) + { + discovered_name_len = MAX_NAME_LENGTH; + } + + bool found_discovered_in_list = 0; + + for(int i = 0; known_controllers[i] != NULL; i++) + { + if(!found_discovered_in_list) { - LOGGER_ERR("error accepting client %s\n", strerror(errno)); - continue; - } - - size_t payload_length; - - if(recv(client_fd, &payload_length, sizeof(payload_length), 0) <= 0) - { - LOGGER_ERR("error receiving header from client\n"); - continue; - } - - char *answer_payload = (char*)malloc((payload_length)); - if(recv(client_fd, answer_payload, payload_length, 0) <= 0) - { - LOGGER_ERR("error receiving payload from client\n"); - continue; - } - - struct sockaddr_in addr; - socklen_t client_addr_size = sizeof(struct sockaddr_in); - if(getpeername(client_fd, (struct sockaddr *)&addr, &client_addr_size) != 0) - { - - LOGGER_ERR("error receiving payload from client\n"); - continue; - } - - LOGGER_DEBUG("received info for discovered controller\n"); - - uuid_t discovered_id; - - mpack_tree_t tree; - mpack_tree_init_data(&tree, answer_payload, payload_length); - mpack_tree_parse(&tree); - mpack_node_t root = mpack_tree_root(&tree); - - memcpy(discovered_id, mpack_node_data(mpack_node_map_uint(root, DISCOVERY_MAPPING_ID)), sizeof(uuid_t)); - - uint16_t discovered_command_port = mpack_node_u16(mpack_node_map_uint(root, DISCOVERY_MAPPING_COMMAND_PORT)); - uint8_t discovered_relay_count = mpack_node_u8(mpack_node_map_uint(root, DISCOVERY_MAPPING_RELAY_COUNT)); - const char *discovered_name = mpack_node_str(mpack_node_map_uint(root, DISCOVERY_MAPPING_NAME)); - size_t discovered_name_len = mpack_node_strlen(mpack_node_map_uint(root, DISCOVERY_MAPPING_NAME)); - - if(discovered_name_len > MAX_NAME_LENGTH) - { - discovered_name_len = MAX_NAME_LENGTH; - } - - bool found_discovered_in_list = 0; - - for(int i = 0; known_controllers[i] != NULL; i++) - { - if(!found_discovered_in_list) + if(uuid_compare(known_controllers[i]->uid, discovered_id) == 0) { - if(uuid_compare(known_controllers[i]->uid, discovered_id) == 0) - { - LOGGER_DEBUG("rediscovered a known controller at %s\n", inet_ntoa(addr.sin_addr)); + LOGGER_DEBUG("rediscovered a known controller at %s\n", inet_ntoa(addr.sin_addr)); - known_controllers[i]->active = 1; - strlcpy(known_controllers[i]->name, discovered_name, discovered_name_len + 1); - strlcpy(known_controllers[i]->ip, inet_ntoa(addr.sin_addr), IP_LENGTH + 1); - known_controllers[i]->name[discovered_name_len] = '\0'; - known_controllers[i]->port = discovered_command_port; - known_controllers[i]->relay_count = discovered_relay_count; + known_controllers[i]->active = 1; + strlcpy(known_controllers[i]->name, discovered_name, discovered_name_len + 1); + strlcpy(known_controllers[i]->ip, inet_ntoa(addr.sin_addr), IP_LENGTH + 1); + known_controllers[i]->name[discovered_name_len] = '\0'; + known_controllers[i]->port = discovered_command_port; + known_controllers[i]->relay_count = discovered_relay_count; - controller_save(known_controllers[i]); - controller_free(known_controllers[i]); + controller_save(known_controllers[i]); + controller_free(known_controllers[i]); - found_discovered_in_list = 1; - known_controllers[i] = known_controllers[i + 1]; - } - } - else - { + found_discovered_in_list = 1; known_controllers[i] = known_controllers[i + 1]; } } - - if(!found_discovered_in_list) + else { - LOGGER_DEBUG("discovered a new controller at %s\n", inet_ntoa(addr.sin_addr)); - - controller_t *discovered_controller = malloc(sizeof(controller_t)); - discovered_controller->id = 0; - strlcpy(discovered_controller->ip, inet_ntoa(addr.sin_addr), IP_LENGTH + 1); - uuid_copy(discovered_controller->uid, discovered_id); - strncpy(discovered_controller->name, discovered_name, discovered_name_len); - discovered_controller->name[discovered_name_len] = '\0'; - discovered_controller->relay_count = discovered_relay_count; - discovered_controller->port = discovered_command_port; - discovered_controller->active = 1; - - controller_save(discovered_controller); - - // TODO get relays during discovery and don't create empty ones - relay_t **discovered_relays = malloc(sizeof(relay_t*) * (discovered_controller->relay_count + 1)); - for(int i = 0; i < discovered_controller->relay_count; ++i) - { - relay_t *new_relay = malloc(sizeof(relay_t)); - new_relay->id = 0; - sprintf(new_relay->name, "Relay %d", i + 1); - new_relay->number = i; - new_relay->controller_id = discovered_controller->id; - - uuid_t tmp_uuid; - memset(tmp_uuid, 0, sizeof(uuid_t)); - memcpy(tmp_uuid, "off", 3); - - for(int i = 0; i < 7; ++i) - { - new_relay->schedules[i] = schedule_get_by_uid(tmp_uuid); - } - time_t timestamp = time(NULL); - struct tm *time_struct = localtime(×tamp); - new_relay->active_schedule = new_relay->schedules[helper_get_weekday(time_struct)]; - - relay_save(new_relay); - - discovered_relays[i] = new_relay; - } - discovered_relays[discovered_controller->relay_count] = NULL; - discovered_controller->relays = discovered_relays; - - controller_free(discovered_controller); + known_controllers[i] = known_controllers[i + 1]; } - mpack_tree_destroy(&tree); - free(answer_payload); - - discover_answer_buf[0] = 0; // TODO add discovery return codes - send(client_fd, discover_answer_buf, sizeof(uint8_t), 0); - close(client_fd); } + + if(!found_discovered_in_list) + { + LOGGER_DEBUG("discovered a new controller at %s\n", inet_ntoa(addr.sin_addr)); + + controller_t *discovered_controller = malloc(sizeof(controller_t)); + discovered_controller->id = 0; + strlcpy(discovered_controller->ip, inet_ntoa(addr.sin_addr), IP_LENGTH + 1); + uuid_copy(discovered_controller->uid, discovered_id); + strncpy(discovered_controller->name, discovered_name, discovered_name_len); + discovered_controller->name[discovered_name_len] = '\0'; + discovered_controller->relay_count = discovered_relay_count; + discovered_controller->port = discovered_command_port; + discovered_controller->active = 1; + + controller_save(discovered_controller); + + // TODO get relays during discovery and don't create empty ones + relay_t **discovered_relays = malloc(sizeof(relay_t*) * (discovered_controller->relay_count + 1)); + for(int i = 0; i < discovered_controller->relay_count; ++i) + { + relay_t *new_relay = malloc(sizeof(relay_t)); + new_relay->id = 0; + sprintf(new_relay->name, "Relay %d", i + 1); + new_relay->number = i; + new_relay->controller_id = discovered_controller->id; + + uuid_t tmp_uuid; + memset(tmp_uuid, 0, sizeof(uuid_t)); + memcpy(tmp_uuid, "off", 3); + + for(int i = 0; i < 7; ++i) + { + new_relay->schedules[i] = schedule_get_by_uid(tmp_uuid); + } + time_t timestamp = time(NULL); + struct tm *time_struct = localtime(×tamp); + new_relay->active_schedule = new_relay->schedules[helper_get_weekday(time_struct)]; + + relay_save(new_relay); + + discovered_relays[i] = new_relay; + } + discovered_relays[discovered_controller->relay_count] = NULL; + discovered_controller->relays = discovered_relays; + + controller_free(discovered_controller); + } + mpack_tree_destroy(&tree); + free(answer_payload); + + discover_answer_buf[0] = 0; // TODO add discovery return codes + send(client_fd, discover_answer_buf, sizeof(uint8_t), 0); + close(client_fd); } for(int i = 0; known_controllers[i] != NULL; i++) { diff --git a/src/endpoints/api_v1_macros_STR.c b/src/endpoints/api_v1_macros_STR.c index fd03267..5c4c5fb 100644 --- a/src/endpoints/api_v1_macros_STR.c +++ b/src/endpoints/api_v1_macros_STR.c @@ -277,5 +277,4 @@ api_v1_macros_STR_DELETE(struct mg_connection *nc, struct http_message *hm, endp M_RESPONSE_MSG(LOGGER_DEBUG, response, 200, "deleted macro"); } macro_free(macro); - return; } diff --git a/src/endpoints/api_v1_schedules_STR.c b/src/endpoints/api_v1_schedules_STR.c index 9862b43..5397e49 100644 --- a/src/endpoints/api_v1_schedules_STR.c +++ b/src/endpoints/api_v1_schedules_STR.c @@ -204,5 +204,4 @@ api_v1_schedules_STR_DELETE(struct mg_connection *nc, struct http_message *hm, e M_RESPONSE_MSG(LOGGER_DEBUG, response, 200, "the target schedule got deleted"); } schedule_free(schedule); - return; } diff --git a/src/endpoints/api_v1_tags.c b/src/endpoints/api_v1_tags.c index 85b403e..cb3891f 100644 --- a/src/endpoints/api_v1_tags.c +++ b/src/endpoints/api_v1_tags.c @@ -95,5 +95,4 @@ api_v1_tags_POST(struct mg_connection *nc, struct http_message *hm, endpoint_arg } cJSON_Delete(json); - return; } diff --git a/src/handlers/http.c b/src/handlers/http.c index 1dd2a7a..5ef3bbd 100644 --- a/src/handlers/http.c +++ b/src/handlers/http.c @@ -118,11 +118,9 @@ handle_http_request(struct mg_connection *nc, struct http_message *hm) LOGGER_DEBUG("serving %.*s\n", hm->uri.len, hm->uri.p); return; } - else - { - LOGGER_DEBUG("serving 'not found'\n"); - endpoint = router_get_not_found_endpoint(); - } + + LOGGER_DEBUG("serving 'not found'\n"); + endpoint = router_get_not_found_endpoint(); } if(endpoint->method == HTTP_METHOD_OPTIONS) diff --git a/src/handlers/mqtt.c b/src/handlers/mqtt.c index 7f26ffc..886e556 100644 --- a/src/handlers/mqtt.c +++ b/src/handlers/mqtt.c @@ -9,7 +9,7 @@ #include static void -handle_mqtt_publish_controller(char **topic_save, int controller_id, char *payload) +handle_mqtt_publish_controller(char **topic_save, int controller_id, const char *payload) { (void)controller_id; (void)payload; diff --git a/src/helpers/connect_server.c b/src/helpers/connect_server.c index db54375..7f01a74 100644 --- a/src/helpers/connect_server.c +++ b/src/helpers/connect_server.c @@ -13,8 +13,11 @@ helper_connect_tcp_server(char* host, uint16_t port) char port_str[6]; sprintf(port_str, "%d", port); - int s, status; - struct addrinfo hints, *res; + int s; + int status; + struct addrinfo *res; + struct addrinfo hints; + memset(&hints, 0, sizeof hints); hints.ai_family = AF_INET; //set IP Protocol flag (IPv4 or IPv6 - we don't care) hints.ai_socktype = SOCK_STREAM; //set socket flag diff --git a/src/models/controller.c b/src/models/controller.c index 013bcfd..f585ff3 100644 --- a/src/models/controller.c +++ b/src/models/controller.c @@ -105,11 +105,9 @@ controller_db_select(sqlite3_stmt *stmt) { break; } - else - { - LOGGER_ERR("error selecting controllers from database: %s\n", sqlite3_errstr(s)); - break; - } + + LOGGER_ERR("error selecting controllers from database: %s\n", sqlite3_errstr(s)); + break; } } sqlite3_finalize(stmt); diff --git a/src/models/macro.c b/src/models/macro.c index c513956..d658ff1 100644 --- a/src/models/macro.c +++ b/src/models/macro.c @@ -82,11 +82,9 @@ macro_db_select(sqlite3_stmt *stmt) { break; } - else - { - LOGGER_ERR("error selecting macros from database: %s\n", sqlite3_errstr(s)); - break; - } + + LOGGER_ERR("error selecting macros from database: %s\n", sqlite3_errstr(s)); + break; } } sqlite3_finalize(stmt); diff --git a/src/models/macro_action.c b/src/models/macro_action.c index 97cbfc6..da4c82d 100644 --- a/src/models/macro_action.c +++ b/src/models/macro_action.c @@ -61,11 +61,9 @@ macro_action_db_select(sqlite3_stmt *stmt) { break; } - else - { - LOGGER_ERR("error selecting macro_actions from database: %s\n", sqlite3_errstr(s)); - break; - } + + LOGGER_ERR("error selecting macro_actions from database: %s\n", sqlite3_errstr(s)); + break; } } sqlite3_finalize(stmt); diff --git a/src/models/period.c b/src/models/period.c index a5a006a..c582164 100644 --- a/src/models/period.c +++ b/src/models/period.c @@ -27,9 +27,8 @@ period_helper_parse_hhmm(const char *hhmm_str, uint16_t *hhmm) } } - uint16_t tmp_h, tmp_m; - tmp_h = (uint16_t)strtol(&hhmm_str[0], NULL, 10); - tmp_m = (uint16_t)strtol(&hhmm_str[3], NULL, 10); + uint16_t tmp_h = (uint16_t)strtol(&hhmm_str[0], NULL, 10); + uint16_t tmp_m = (uint16_t)strtol(&hhmm_str[3], NULL, 10); if(tmp_h > 24 || tmp_m >= 60) { diff --git a/src/router.c b/src/router.c index ae6a0c5..1831b67 100644 --- a/src/router.c +++ b/src/router.c @@ -314,7 +314,6 @@ router_find_endpoint(const char *uri_str, size_t uri_len, struct mg_str *method_ continue; } endpoints[i].possible_route = 0; - continue; } uri_token = strtok_r(NULL, delimiter, &uri_token_save); ++route_part;