Fix some more memory issues and code organization issues

PiHelper/cli.c

@@ -22,7 +22,9 @@
 #include <string.h>
 #include <unistd.h>
 #include "cli.h"
-#include "pihelper.h"
+#include "log.h"
+
+static char * DEFAULT_CONFIG_PATH = "/.config/pihelper.conf";
 
 int main(int argc, char ** argv) {
 
@@ -87,10 +89,12 @@ int main(int argc, char ** argv) {
         config_path[path_len] = '\0';
     }
     if (access(config_path, F_OK)) {
-        char * user_input = malloc(2);
+        char * user_input = malloc(4);
         // Intentionally using printf here to ensure that this is always printed
         printf("No Pi-Helper configuration found. Would you like to create it now? [Y/n] ");
-        fgets(user_input, 2, stdin);
+        fgets(user_input, 3, stdin);
+        user_input[3] = '\0';
+        write_log(PIHELPER_LOG_DEBUG, "User's input: %s", user_input);
         if (strstr(user_input, "\n") == user_input
                 || strstr(user_input, "Y") == user_input
                 || strstr(user_input, "y") == user_input
@@ -108,9 +112,11 @@ int main(int argc, char ** argv) {
 
     pihole_config * config;
     if (configure) {
+        write_log(PIHELPER_LOG_DEBUG, "Configuring PiHelper");
         config = configure_pihole(config_path);
     } else {
-        config = read_config(config_path);
+        write_log(PIHELPER_LOG_DEBUG, "Reading existing PiHelper config");
+        config = pihelper_read_config(config_path);
     }
     int retval;
     if (config == NULL) {
@@ -118,14 +124,14 @@ int main(int argc, char ** argv) {
         retval = 1;
     } else if (enable && disable != NULL) {
         print_usage();
-        retval =  PIHELPER_INVALID_COMMANDS;
+        retval = PIHELPER_INVALID_COMMANDS;
     } else if (enable) {
-        retval = enable_pihole(config);
+        retval = pihelper_enable_pihole(config);
     } else if (disable != NULL) {
-        retval =  disable_pihole(config, disable);
+        retval = pihelper_disable_pihole(config, disable);
         free(disable);
     } else {
-        retval = get_status(config);
+        retval = pihelper_get_status(config);
     }
 
     free(config_path);
@@ -144,3 +150,28 @@ void print_usage() {
     printf(" -v, --verbose                Print debug logs\n");
 }
 
+pihole_config * configure_pihole(char * config_path) {
+    if (access(config_path, F_OK) == 0) {
+        // TODO: Check if file is accessible for read/write (not just if it exists)
+        write_log(PIHELPER_LOG_WARN, "WARNING: The config file already exists. Continuing will overwrite any existing configuration.\n");
+    }
+    pihole_config * config = pihelper_new_config();
+    printf("Enter the hostname or ip address for your pi-hole: ");
+    char * host = calloc(1, 257);
+    fgets(host, 256, stdin);
+    host[256] = '\0';
+    write_log(PIHELPER_LOG_DEBUG, "User entered \"%s\" for host", host);
+    pihelper_config_set_host(config, host);
+    free(host);
+    char * raw_pass = getpass("Enter the api key or web password for your pi-hole: ");
+    if (strlen(raw_pass) != 64) {
+        pihelper_config_set_password(config, raw_pass);
+    } else {
+        pihelper_config_set_api_key(config, raw_pass);
+    }
+    free(raw_pass);
+    // TODO: Make an authenticated request to verify that the credentials are valid and save the config
+    pihelper_save_config(config, config_path);
+    return config;
+}
+

PiHelper/cli.h

@@ -17,6 +17,7 @@
  * along with PiHelper.  If not, see <https://www.gnu.org/licenses/>.
  */
 #include <getopt.h>
+#include "pihelper.h"
 
 static char * shortopts = "cd::ef:hqv";
 
@@ -32,3 +33,5 @@ static struct option longopts[] = {
 
 void print_usage();
 
+pihole_config * configure_pihole(char * config_path);
+

PiHelper/config.c

@@ -25,6 +25,8 @@
 #include <unistd.h>
 #include "config.h"
 
+const int MAX_PIHOLE_API_KEY = 64;
+
 int mkdirs(char * path) {
     char * curPos = strstr(path, "/") + 1;
     char parents[strlen(path)];
@@ -41,10 +43,10 @@ int mkdirs(char * path) {
     return retval;
 }
 
-void save_config(pihole_config * config, char * config_path) {
+int save_config(pihole_config * config, char * config_path) {
     if (mkdirs(config_path)) {
         perror(config_path);
-        exit(1);
+        return 1;
     }
     FILE * config_file = fopen(config_path, "w");
     int config_len = strlen(config->host) + strlen(config->api_key) + 16;
@@ -53,6 +55,7 @@ void save_config(pihole_config * config, char * config_path) {
     config_string[config_len + 1] = '\0';
     fputs(config_string, config_file);
     fclose(config_file);
+    return 0;
 }
 
 /*
@@ -61,12 +64,12 @@ void save_config(pihole_config * config, char * config_path) {
 static char * hash_string (char * raw_string) {
     unsigned char bytes[SHA256_DIGEST_LENGTH];
     SHA256((unsigned char *) raw_string, strlen(raw_string), bytes);
-    char * hash = malloc(65);
+    char * hash = malloc(MAX_PIHOLE_API_KEY + 1);
     int i;
     for(i = 0; i < SHA256_DIGEST_LENGTH; i++) {
         sprintf(hash + (i * 2), "%02x", bytes[i]);
     }
-    hash[64] = '\0';
+    hash[MAX_PIHOLE_API_KEY] = '\0';
     return hash;
 }
 
@@ -76,22 +79,21 @@ pihole_config * read_config(char * config_path) {
         return NULL;
     }
 
-    pihole_config * config = calloc(1, sizeof(pihole_config));
     FILE * config_file = fopen(config_path, "r");
-    char host[_POSIX_HOST_NAME_MAX + 7];
+    char * host = calloc(1, _POSIX_HOST_NAME_MAX + 7);
     fgets(host, _POSIX_HOST_NAME_MAX + 7, config_file);
     if (strstr(host, "host=") == NULL || strlen(host) < 7) {
         write_log(PIHELPER_LOG_DEBUG, "Config file contains invalid host: %s", host);
         write_log(PIHELPER_LOG_ERROR, "Invalid config file");
-        free_config(config);
         fclose(config_file);
         return NULL;
     }
-    config->host = calloc(1, strlen(host) - 5);
+    pihole_config * config = pihole_config_new();
-    strncpy(config->host, host + 5, strlen(host) - 6);
+    config_set_host(config, host + 5);
-    config->host[strlen(host) - 6] = '\0';
+    free(host);
     char * api_key = calloc(1, 74);
     fgets(api_key, 74, config_file);
+    fclose(config_file);
     if (strstr(api_key, "api-key=") == NULL
             || strlen(api_key) < 9) {
         write_log(PIHELPER_LOG_DEBUG, "Config file contains invalid api key: %s", api_key);
@@ -100,45 +102,64 @@ pihole_config * read_config(char * config_path) {
         fclose(config_file);
         return config;
     }
-    config->api_key = calloc(1, strlen(api_key) - 8);
+    config_set_api_key(config, api_key + 8);
-    strncpy(config->api_key, api_key + 8, strlen(api_key) - 9);
-    config->api_key[strlen(api_key) - 9] = '\0';
     free(api_key);
-    fclose(config_file);
     write_log(PIHELPER_LOG_DEBUG, "Using host %s and api key %s", config->host, config->api_key);
     return config;
 }
 
-pihole_config * configure_pihole(char * config_path) {
+pihole_config * pihole_config_new() {
-    if (access(config_path, F_OK) == 0) {
+    pihole_config * config;
-        // TODO: Check if file is accessible for read/write (not just if it exists)
+    if ((config = malloc(sizeof(pihole_config))) == NULL) {
-        write_log(PIHELPER_LOG_WARN, "WARNING: The config file already exists. Continuing will overwrite any existing configuration.\n");
+        write_log(PIHELPER_LOG_ERROR, "Failed to allocate memory for config");
+        free_config(config);
+        return NULL;
     }
-    pihole_config * config = malloc(sizeof(pihole_config));
+    if ((config->host = calloc(1, _POSIX_HOST_NAME_MAX + 1)) == NULL) {
-    config->host = calloc(1, _POSIX_HOST_NAME_MAX);
+        write_log(PIHELPER_LOG_ERROR, "Failed to allocate memory for config host");
-    config->host[_POSIX_HOST_NAME_MAX - 1] = '\0';
+        free_config(config);
-    // Intentionally using printf to ensure this is always printed
+        return NULL;
-    printf("Enter the hostname or ip address for your pi-hole: ");
-    fgets(config->host, _POSIX_HOST_NAME_MAX, stdin);
-    char * newline = strstr(config->host, "\n");
-    if (newline != NULL) {
-        config->host[strlen(config->host) - strlen(newline)] = '\0';
     }
-    config->api_key = getpass("Enter the api key or web password for your pi-hole: ");
+    if ((config->api_key = calloc(1, MAX_PIHOLE_API_KEY + 1)) == NULL) {
-    if (strlen(config->api_key) != 64) {
+        write_log(PIHELPER_LOG_ERROR, "Failed to allocate memory for config API key");
-        // This is definitely not an API key, so hash it
+        free_config(config);
-        // The Pi-hole hashes twice so we do the same here
+        return NULL;
-        char * first = hash_string(config->api_key);
-        char * hash = hash_string(first);
-        free(first);
-        free(config->api_key);
-        config->api_key = hash;
     }
-    // TODO: Make an authenticated request to verify that the credentials are valid and save the config
+    config->host[_POSIX_HOST_NAME_MAX] = '\0';
-    save_config(config, config_path);
+    config->api_key[MAX_PIHOLE_API_KEY] = '\0';
     return config;
 }
 
+void config_set_host(pihole_config * config, char * host) {
+    strncpy(config->host, host, _POSIX_HOST_NAME_MAX);
+    config->host[_POSIX_HOST_NAME_MAX] = '\0';
+    trim_string(config->host);
+}
+
+void config_set_password(pihole_config * config, char * password) {
+    trim_string(password);
+    // This is definitely not an API key, so hash it
+    // The Pi-hole hashes twice so we do the same here
+    char * first = hash_string(password);
+    char * hash = hash_string(first);
+    free(first);
+    config_set_api_key(config, hash);
+    free(hash);
+}
+
+void config_set_api_key(pihole_config * config, char * api_key) {
+    strncpy(config->api_key, api_key, MAX_PIHOLE_API_KEY);
+    config->api_key[MAX_PIHOLE_API_KEY] = '\0';
+    trim_string(config->api_key);
+}
+
+static void trim_string(char * raw_str) {
+    char * newline = strstr(raw_str, "\n");
+    if (newline != NULL) {
+        raw_str[strlen(raw_str) - strlen(newline)] = '\0';
+    }
+}
+
 void free_config(pihole_config * config) {
     if (config == NULL) return;
     if (config->host != NULL) {

PiHelper/config.h

@@ -20,20 +20,22 @@
 #define PIHELPER_CONFIG
 #include <openssl/sha.h>
 #include "log.h"
+#include "pihelper.h"
 
-static char * DEFAULT_CONFIG_PATH = "/.config/pihelper.conf";
+int save_config(pihole_config * config, char * config_path);
-
-typedef struct {
-    char * host;
-    char * api_key;
-} pihole_config;
-
-void save_config(pihole_config * config, char * config_path);
 
 pihole_config * read_config(char * config_path);
 
-pihole_config * configure_pihole(char * config_path);
+pihole_config * pihole_config_new();
+
+void config_set_host(pihole_config * config, char * host);
+
+void config_set_password(pihole_config * config, char * password);
+
+void config_set_api_key(pihole_config * config, char * api_key);
 
 void free_config(pihole_config * config);
+
+static void trim_string(char * raw_str);
 #endif
 

PiHelper/log.c

@@ -22,6 +22,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include "log.h"
+#include "pihelper.h"
 
 int LOG_LEVEL = 2; // Default to info logs
 

PiHelper/log.h

@@ -18,12 +18,7 @@
  */
 #ifndef PIHELPER_LOG
 #define PIHELPER_LOG
-
+extern int LOG_LEVEL;
-static int PIHELPER_LOG_DISABLED = -1;
-static int PIHELPER_LOG_ERROR    =  0;
-static int PIHELPER_LOG_WARN     =  1;
-static int PIHELPER_LOG_INFO     =  2;
-static int PIHELPER_LOG_DEBUG    =  3;
 
 void set_log_level(int level);
 

PiHelper/network.c

@@ -26,13 +26,13 @@
 #include "log.h"
 #include "network.h"
 
-static char * URL_FORMAT = "http://%s/admin/api.php";
+static char * URL_FORMAT     = "http://%s/admin/api.php";
-static int URL_FORMAT_LEN = 22;
+static int    URL_FORMAT_LEN = 22;
-static char * AUTH_QUERY = "auth";
+static char * AUTH_QUERY     = "auth";
-static char * ENABLE_QUERY = "enable";
+static char * ENABLE_QUERY   = "enable";
-static char * DISABLE_QUERY = "disable";
+static char * DISABLE_QUERY  = "disable";
-static char * HTTP_SCHEME = "http://";
+static char * HTTP_SCHEME    = "http://";
-static char * HTTPS_SCHEME = "https://";
+static char * HTTPS_SCHEME   = "https://";
 
 int get_status(pihole_config * config) {
     write_log(PIHELPER_LOG_DEBUG, "Getting Pi-hole status…");
@@ -111,11 +111,16 @@ static char * get(char endpoint[]) {
     curl_easy_setopt(curl, CURLOPT_URL, endpoint);
     curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, receive_data);
     curl_easy_setopt(curl, CURLOPT_WRITEDATA, &response);
+    curl_easy_setopt(curl, CURLOPT_TIMEOUT, 10);
+    if (LOG_LEVEL == PIHELPER_LOG_DEBUG) {
+        curl_easy_setopt(curl, CURLOPT_VERBOSE, 1);
+    }
     int res = curl_easy_perform(curl);
     curl_easy_cleanup(curl);
     if (res == CURLE_OK) {
         return response.body;
     } else {
+        free(response.body);
         return NULL;
     }
 }
@@ -151,8 +156,10 @@ static void parse_status(char * raw_json) {
     } while ((jerr = json_tokener_get_error(tok)) == json_tokener_continue);
     if (jerr != json_tokener_success) {
         write_log(PIHELPER_LOG_ERROR, "Failed to parse JSON: %s", json_tokener_error_desc(jerr));
+        json_tokener_free(tok);
         return;
     }
+    write_log(PIHELPER_LOG_DEBUG, "%s", json_object_to_json_string_ext(jobj, JSON_C_TO_STRING_PRETTY));
     json_object *status;
     const char * status_string;
     if (json_pointer_get(jobj, "/status", &status) == 0

PiHelper/pihelper.c

@@ -16,6 +16,9 @@
  * You should have received a copy of the GNU General Public License
  * along with PiHelper.  If not, see <https://www.gnu.org/licenses/>.
  */
+#include "config.h"
+#include "log.h"
+#include "network.h"
 #include "pihelper.h"
 
 int pihelper_get_status(pihole_config * config) {
@@ -34,6 +37,30 @@ void pihelper_set_log_level(int level) {
     set_log_level(level);
 }
 
+pihole_config * pihelper_new_config() {
+    return pihole_config_new();
+}
+
+void pihelper_config_set_host(pihole_config * config, char * host) {
+    config_set_host(config, host);
+}
+
+void pihelper_config_set_password(pihole_config * config, char * password) {
+    config_set_password(config, password);
+}
+
+void pihelper_config_set_api_key(pihole_config * config, char * api_key) {
+    config_set_api_key(config, api_key);
+}
+
+pihole_config * pihelper_read_config(char * config_path) {
+    read_config(config_path);
+}
+
+int pihelper_save_config(pihole_config * config, char * config_path) {
+    save_config(config, config_path);
+}
+
 void pihelper_free_config(pihole_config * config) {
     free_config(config);
 }

PiHelper/pihelper.h

@@ -18,13 +18,22 @@
  */
 #ifndef PIHELPER
 #define PIHELPER
-#include "config.h"
-#include "log.h"
-#include "network.h"
 static int PIHELPER_OK = 0;
 static int PIHELPER_HELP = 1;
 static int PIHELPER_INVALID_COMMANDS = 2;
 
+static int PIHELPER_LOG_DISABLED = -1;
+static int PIHELPER_LOG_ERROR    =  0;
+static int PIHELPER_LOG_WARN     =  1;
+static int PIHELPER_LOG_INFO     =  2;
+static int PIHELPER_LOG_DEBUG    =  3;
+
+
+typedef struct {
+    char * host;
+    char * api_key;
+} pihole_config;
+
 void pihelper_set_log_level(int level);
 
 int pihelper_get_status(pihole_config * config);
@@ -33,6 +42,18 @@ int pihelper_enable_pihole(pihole_config * config);
 
 int pihelper_disable_pihole(pihole_config * config, char * duration);
 
+pihole_config * pihelper_new_config();
+
+void pihelper_config_set_host(pihole_config * config, char * host);
+
+void pihelper_config_set_password(pihole_config * config, char * password);
+
+void pihelper_config_set_api_key(pihole_config * config, char * api_key);
+
+pihole_config * pihelper_read_config(char * config_path);
+
+int pihelper_save_config(pihole_config * config, char * config_path);
+
 void pihelper_free_config(pihole_config * config);
 #endif