diff --git a/src/FTL.h b/src/FTL.h index d6a7f1c37..64986a20e 100644 --- a/src/FTL.h +++ b/src/FTL.h @@ -152,7 +152,7 @@ // caused by insufficient memory or by code bugs (not properly dealing // with NULL pointers) much easier. #undef strdup // strdup() is a macro in itself, it needs special handling -#define free(ptr) FTLfree((void**)&ptr, __FILE__, __FUNCTION__, __LINE__) +#define free(ptr) { FTLfree(ptr, __FILE__, __FUNCTION__, __LINE__); ptr = NULL; } #define strdup(str_in) FTLstrdup(str_in, __FILE__, __FUNCTION__, __LINE__) #define calloc(numer_of_elements, element_size) FTLcalloc(numer_of_elements, element_size, __FILE__, __FUNCTION__, __LINE__) #define realloc(ptr, new_size) FTLrealloc(ptr, new_size, __FILE__, __FUNCTION__, __LINE__) diff --git a/src/config/password.c b/src/config/password.c index ee6e102ff..ddae3856c 100644 --- a/src/config/password.c +++ b/src/config/password.c @@ -440,7 +440,6 @@ enum password_result verify_password(const char *password, const char *pwhash, c config.webserver.api.pwhash.v.s = new_hash; config.webserver.api.pwhash.t = CONF_STRING_ALLOCATED; writeFTLtoml(true); - free(new_hash); } // Successful logins do not count against rate-limiting diff --git a/src/config/setupVars.c b/src/config/setupVars.c index cd5df0ea8..57dfd8180 100644 --- a/src/config/setupVars.c +++ b/src/config/setupVars.c @@ -122,7 +122,6 @@ static void get_conf_bool_from_setupVars(const char *key, struct conf_item *conf static void get_revServer_from_setupVars(void) { - char *active = NULL; char *cidr = NULL; char *target = NULL; char *domain = NULL; @@ -144,7 +143,7 @@ static void get_revServer_from_setupVars(void) clearSetupVarsArray(); return; } - active = strdup(active_str); + bool active = strcasecmp(active_str, "true") == 0; // Free memory, harmless to call if read_setupVarsconf() didn't return a result clearSetupVarsArray(); @@ -186,16 +185,16 @@ static void get_revServer_from_setupVars(void) clearSetupVarsArray(); // Only add the entry if all values are present and active - if(active != NULL && cidr != NULL && target != NULL && domain != NULL) + if(cidr != NULL && target != NULL && domain != NULL) { // Build comma-separated string of all values // 9 = 3 commas, "true/false", and null terminator char *old = calloc(strlen(cidr) + strlen(target) + strlen(domain) + 9, sizeof(char)); - if(old) + if(old != NULL) { // Add to new config // active is always true as we only add active entries - sprintf(old, "%s,%s,%s,%s", active_str, cidr, target, domain); + sprintf(old, "%s,%s,%s,%s", active ? "true" : "false", cidr, target, domain); cJSON_AddItemToArray(config.dns.revServers.v.json, cJSON_CreateString(old)); // Parameter present in setupVars.conf @@ -211,8 +210,6 @@ static void get_revServer_from_setupVars(void) } // Free memory - if(active != NULL) - free(active); if(cidr != NULL) free(cidr); if(target != NULL) diff --git a/src/config/toml_helper.c b/src/config/toml_helper.c index dabc1f594..37cf792ad 100644 --- a/src/config/toml_helper.c +++ b/src/config/toml_helper.c @@ -543,7 +543,7 @@ void readTOMLvalue(struct conf_item *conf_item, const char* key, toml_table_t *t } case CONF_ENUM_PTR_TYPE: { - const toml_datum_t val = toml_string_in(toml, key); + toml_datum_t val = toml_string_in(toml, key); if(val.ok) { const int ptr_type = get_ptr_type_val(val.u.s); @@ -559,7 +559,7 @@ void readTOMLvalue(struct conf_item *conf_item, const char* key, toml_table_t *t } case CONF_ENUM_BUSY_TYPE: { - const toml_datum_t val = toml_string_in(toml, key); + toml_datum_t val = toml_string_in(toml, key); if(val.ok) { const int busy_reply = get_busy_reply_val(val.u.s); @@ -575,7 +575,7 @@ void readTOMLvalue(struct conf_item *conf_item, const char* key, toml_table_t *t } case CONF_ENUM_BLOCKING_MODE: { - const toml_datum_t val = toml_string_in(toml, key); + toml_datum_t val = toml_string_in(toml, key); if(val.ok) { const int blocking_mode = get_blocking_mode_val(val.u.s); @@ -591,7 +591,7 @@ void readTOMLvalue(struct conf_item *conf_item, const char* key, toml_table_t *t } case CONF_ENUM_REFRESH_HOSTNAMES: { - const toml_datum_t val = toml_string_in(toml, key); + toml_datum_t val = toml_string_in(toml, key); if(val.ok) { const int refresh_hostnames = get_refresh_hostnames_val(val.u.s); @@ -607,7 +607,7 @@ void readTOMLvalue(struct conf_item *conf_item, const char* key, toml_table_t *t } case CONF_ENUM_LISTENING_MODE: { - const toml_datum_t val = toml_string_in(toml, key); + toml_datum_t val = toml_string_in(toml, key); if(val.ok) { const int listeningMode = get_listeningMode_val(val.u.s); @@ -623,7 +623,7 @@ void readTOMLvalue(struct conf_item *conf_item, const char* key, toml_table_t *t } case CONF_ENUM_WEB_THEME: { - const toml_datum_t val = toml_string_in(toml, key); + toml_datum_t val = toml_string_in(toml, key); if(val.ok) { const int web_theme = get_web_theme_val(val.u.s); @@ -639,7 +639,7 @@ void readTOMLvalue(struct conf_item *conf_item, const char* key, toml_table_t *t } case CONF_ENUM_TEMP_UNIT: { - const toml_datum_t val = toml_string_in(toml, key); + toml_datum_t val = toml_string_in(toml, key); if(val.ok) { const int temp_unit = get_temp_unit_val(val.u.s); @@ -665,7 +665,7 @@ void readTOMLvalue(struct conf_item *conf_item, const char* key, toml_table_t *t case CONF_STRUCT_IN_ADDR: { struct in_addr addr4 = { 0 }; - const toml_datum_t val = toml_string_in(toml, key); + toml_datum_t val = toml_string_in(toml, key); if(val.ok) { if(strlen(val.u.s) == 0) @@ -686,7 +686,7 @@ void readTOMLvalue(struct conf_item *conf_item, const char* key, toml_table_t *t case CONF_STRUCT_IN6_ADDR: { struct in6_addr addr6 = { 0 }; - const toml_datum_t val = toml_string_in(toml, key); + toml_datum_t val = toml_string_in(toml, key); if(val.ok) { if(strlen(val.u.s) == 0) @@ -717,7 +717,7 @@ void readTOMLvalue(struct conf_item *conf_item, const char* key, toml_table_t *t for(unsigned int i = 0; i < nelem; i++) { // Get string from TOML - const toml_datum_t d = toml_string_at(array, i); + toml_datum_t d = toml_string_at(array, i); if(!d.ok) { log_warn("Config %s is an invalid array (found at index %u)", conf_item->k, i); diff --git a/src/signals.c b/src/signals.c index 392aff3ab..35971f0db 100644 --- a/src/signals.c +++ b/src/signals.c @@ -29,7 +29,7 @@ #define BINARY_NAME "pihole-FTL" volatile sig_atomic_t killed = 0; -static volatile pid_t mpid = -1; +static volatile pid_t mpid = 0; static time_t FTLstarttime = 0; volatile int exit_code = EXIT_SUCCESS; @@ -253,7 +253,7 @@ static void __attribute__((noreturn)) signal_handler(int sig, siginfo_t *si, voi log_info("Thank you for helping us to improve our FTL engine!"); // Terminate main process if crash happened in a TCP worker - if(mpid != getpid()) + if(main_pid() != getpid()) { // This is a forked process log_info("Asking parent pihole-FTL (PID %i) to shut down", (int)mpid); @@ -474,7 +474,7 @@ void handle_realtime_signals(void) // Return PID of the main FTL process pid_t main_pid(void) { - if(mpid > -1) + if(mpid > 0) // Has already been set return mpid; else diff --git a/src/syscalls/free.c b/src/syscalls/free.c index 360170eaa..73c9c905e 100644 --- a/src/syscalls/free.c +++ b/src/syscalls/free.c @@ -13,26 +13,20 @@ #include "log.h" #undef free -void FTLfree(void **ptr, const char *file, const char *func, const int line) +bool FTLfree(void *ptr, const char *file, const char *func, const int line) { // The free() function frees the memory space pointed to by ptr, which // must have been returned by a previous call to malloc(), calloc(), or // realloc(). Otherwise, or if free(ptr) has already been called before, // undefined behavior occurs. If ptr is NULL, no operation is performed. if(ptr == NULL) - { - log_warn("Trying to free NULL memory location in %s() (%s:%i)", func, file, line); - return; - } - if(*ptr == NULL) { log_warn("Trying to free NULL pointer in %s() (%s:%i)", func, file, line); - return; + return false; } // Actually free the memory - free(*ptr); + free(ptr); - // Set the pointer to NULL - *ptr = NULL; + return true; } diff --git a/src/syscalls/syscalls.h b/src/syscalls/syscalls.h index 77e7725cb..02024a844 100644 --- a/src/syscalls/syscalls.h +++ b/src/syscalls/syscalls.h @@ -14,7 +14,7 @@ char *FTLstrdup(const char *src, const char *file, const char *func, const int line) __attribute__((malloc)); void *FTLcalloc(size_t n, size_t size, const char *file, const char *func, const int line) __attribute__((malloc)) __attribute__((alloc_size(1,2))); void *FTLrealloc(void *ptr_in, size_t size, const char *file, const char *func, const int line) __attribute__((alloc_size(2))); -void FTLfree(void **ptr, const char*file, const char *func, const int line); +bool FTLfree(void *ptr, const char*file, const char *func, const int line); int FTLfallocate(const int fd, const off_t offset, const off_t len, const char *file, const char *func, const int line); diff --git a/src/webserver/webserver.c b/src/webserver/webserver.c index 745a63b70..b3b05b94a 100644 --- a/src/webserver/webserver.c +++ b/src/webserver/webserver.c @@ -303,6 +303,7 @@ unsigned short get_api_string(char **buf, const bool domain) if(this_len < 0) { log_err("Failed to append API URL to buffer: %s", strerror(errno)); + free(api_str); return 0; } @@ -311,6 +312,7 @@ unsigned short get_api_string(char **buf, const bool domain) if((size_t)this_len >= bufsz - len - 1) { log_err("API URL buffer too small!"); + free(api_str); return 0; } @@ -318,8 +320,8 @@ unsigned short get_api_string(char **buf, const bool domain) if(memmem(*buf, len, api_str, this_len) != NULL) { // This string is already present, so skip it - free(api_str); log_debug(DEBUG_API, "Skipping duplicate API URL: %s", api_str); + free(api_str); continue; }