Merge pull request #109 from marcransome/error-handling

Refactor error handling logic

src/CMakeLists.txt

@@ -1,6 +1,6 @@
 set(target flog)
 
-add_executable(flog main.c flog.c flog.h config.c config.h defs.h utils.c utils.h)
+add_executable(flog main.c flog.c flog.h config.c config.h common.h common.c)
 
 target_link_libraries(${target} PRIVATE ${POPT_LINK_LIBRARIES})
 target_include_directories(${target} PRIVATE ${POPT_INCLUDE_DIRS})

src/utils.c → src/common.c

@@ -20,10 +20,30 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 // SOFTWARE.
 
-#include "utils.h"
-#include "defs.h"
+#include "common.h"
 #include <stdio.h>
 
+static const char *
+flog_error_map[] = {
+    [FLOG_ERROR_NONE]   = "none",
+    [FLOG_ERROR_ALLOC]  = "allocation failed",
+    [FLOG_ERROR_APPEND] = "unable to append log message to file",
+    [FLOG_ERROR_LVL]    = "unknown log level",
+    [FLOG_ERROR_MSG]    = "message string required",
+    [FLOG_ERROR_SUBSYS] = "category option requires subsystem option to be set",
+    [FLOG_ERROR_OPTS]   = "invalid options",
+};
+
+const char *
+flog_error_string(FlogError error) {
+    return flog_error_map[error];
+}
+
+void
+flog_print_error(FlogError error) {
+    fprintf(stderr, "%s: %s\n", PROGRAM_NAME, flog_error_string(error));
+}
+
 void
 flog_usage(void) {
     printf(

src/defs.h → src/common.h

@@ -20,17 +20,48 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 // SOFTWARE.
 
-#ifndef FLOG_DEFS_H
-#define FLOG_DEFS_H
+#ifndef FLOG_COMMON_H
+#define FLOG_COMMON_H
+
+/*! \file common.h
+ *
+ *  Constants, helper functions, and error handling types for common operations.
+ */
 
 #define PROGRAM_NAME "flog"
 #define PROGRAM_VERSION "1.4.0"
 
-#define ERR_CATEGORY_OPTION_REQUIRES_SUBSYSTEM  1
-#define ERR_NO_MESSAGE_STRING_PROVIDED          2
-#define ERR_NO_ARGUMENTS_PROVIDED               3
-#define ERR_PROGRAM_OPTIONS                     4
-#define ERR_CONFIG_ALLOCATION                   5
-#define ERR_FLOG_ALLOCATION                     6
+/*! \brief An enumerated type representing error conditions. */
+typedef enum FlogErrorData {
+    FLOG_ERROR_NONE,
+    FLOG_ERROR_ALLOC,
+    FLOG_ERROR_APPEND,
+    FLOG_ERROR_LVL,
+    FLOG_ERROR_MSG,
+    FLOG_ERROR_OPTS,
+    FLOG_ERROR_SUBSYS,
+} FlogError;
+
+/*! \brief Print usage information to stdout stream. */
+void flog_usage(void);
+
+/*! \brief Print version string to stdout stream. */
+void flog_version(void);
+
+/*! \brief Return a string representation of an error condition.
+ *
+ * \param[in] error A FlogError enumeration variant
+ *
+ * \return A pointer to a null-terminated string describing the error condition
+ */
+const char * flog_get_error_string(FlogError error);
+
+/*! \brief Print a formatted error message representation of the error condition.
+ *
+ * \param[in] error A FlogError enumeration variant representing the error condition
+ *
+ * \return void
+ */
+void flog_print_error(FlogError error);
 
-#endif //FLOG_DEFS_H
+#endif //FLOG_COMMON_H

src/config.c

@@ -21,10 +21,9 @@
 // SOFTWARE.
 
 #include "config.h"
-#include "defs.h"
+#include "common.h"
 #include <stdlib.h>
 #include <stdio.h>
-#include <errno.h>
 #include <sys/syslimits.h>
 #include <string.h>
 #include <assert.h>
@@ -70,22 +69,20 @@ struct FlogConfigData {
 };
 
 FlogConfig *
-flog_config_new(int argc, char *argv[]) {
+flog_config_new(int argc, char *argv[], FlogError *error) {
     assert(argc > 0);
     assert(argv != NULL);
+    assert(error != NULL);
 
-    if (argc == 1) {
-        errno = ERR_NO_ARGUMENTS_PROVIDED;
-        return NULL;
-    }
+    *error = FLOG_ERROR_NONE;
 
     FlogConfig *config = calloc(1, sizeof(struct FlogConfigData));
     if (config == NULL) {
-        errno = ERR_CONFIG_ALLOCATION;
+        *error = FLOG_ERROR_ALLOC;
         return NULL;
     }
 
-    flog_config_set_level(config, Default);
+    flog_config_set_level(config, LVL_DEFAULT);
     flog_config_set_message_type(config, Public);
     flog_config_set_version_flag(config, false);
     flog_config_set_help_flag(config, false);
@@ -111,9 +108,16 @@ flog_config_new(int argc, char *argv[]) {
                 break;
             case 'l':
                 flog_config_set_level(config, flog_config_parse_level(option_argument));
+                if (flog_config_get_level(config) == LVL_UNKNOWN) {
+                    flog_config_free(config);
+                    poptFreeContext(context);
+                    *error = FLOG_ERROR_LVL;
+                    return NULL;
+                }
                 break;
             case 's':
                 flog_config_set_subsystem(config, option_argument);
+
                 break;
             case 'c':
                 flog_config_set_category(config, option_argument);
@@ -134,15 +138,14 @@ flog_config_new(int argc, char *argv[]) {
 
         flog_config_free(config);
         poptFreeContext(context);
-        errno = ERR_PROGRAM_OPTIONS;
+        *error = FLOG_ERROR_OPTS;
         return NULL;
     }
 
     if (strlen(flog_config_get_category(config)) > 0 && strlen(flog_config_get_subsystem(config)) == 0) {
-        fprintf(stderr, "%s: category option requires subsystem option\n", PROGRAM_NAME);
         flog_config_free(config);
         poptFreeContext(context);
-        errno = ERR_CATEGORY_OPTION_REQUIRES_SUBSYSTEM;
+        *error = FLOG_ERROR_SUBSYS;
         return NULL;
     }
 
@@ -151,7 +154,7 @@ flog_config_new(int argc, char *argv[]) {
         if (message_args == NULL) {
             flog_config_free(config);
             poptFreeContext(context);
-            errno = ERR_NO_MESSAGE_STRING_PROVIDED;
+            *error = FLOG_ERROR_MSG;
             return NULL;
         }
 
@@ -243,17 +246,17 @@ flog_config_parse_level(const char *str) {
     FlogConfigLevel level;
 
     if (strcmp(str, "default") == 0) {
-        level = Default;
+        level = LVL_DEFAULT;
     } else if (strcmp(str, "info") == 0) {
-        level = Info;
+        level = LVL_INFO;
     } else if (strcmp(str, "debug") == 0) {
-        level = Debug;
+        level = LVL_DEBUG;
     } else if (strcmp(str, "error") == 0) {
-        level = Error;
+        level = LVL_ERROR;
     } else if (strcmp(str, "fault") == 0) {
-        level = Fault;
+        level = LVL_FAULT;
     } else {
-        level = Unknown;
+        level = LVL_UNKNOWN;
     }
 
     return level;

src/config.h

@@ -31,15 +31,16 @@
 #include <stddef.h>
 #include <stdbool.h>
 #include <stdio.h>
+#include "common.h"
 
 /*! \brief An enumerated type representing the log level. */
 typedef enum FlogConfigLevelData {
-    Default,
-    Info,
-    Debug,
-    Error,
-    Fault,
-    Unknown
+    LVL_DEFAULT,
+    LVL_INFO,
+    LVL_DEBUG,
+    LVL_ERROR,
+    LVL_FAULT,
+    LVL_UNKNOWN
 } FlogConfigLevel;
 
 /*! \brief An enumerated type representing the log message type. */
@@ -57,18 +58,21 @@ typedef struct FlogConfigData FlogConfig;
 /*! \brief Create a FlogConfig object representing configuration to be used
  *         with a FlogCli logger object.
  *
- *  \param argc An integer representing the number of command-line arguments
- *  \param argv A pointer to an array of null-terminated strings representing
- *              command-line arguments
+ *  \param[in]  argc  An integer representing the number of command-line arguments
+ *  \param[in]  argv  A pointer to an array of null-terminated strings representing
+ *                    command-line arguments
+ *  \param[out] error A pointer to a FlogError object that will be used to represent
+ *                    an error condition on failure
  *
  *  \pre \c argc is greater than \c 0
  *  \pre \c argv is \e not \c NULL
+ *  \pre \c error is \e not \c NULL
  *
  *  \return If successful, a pointer to a FlogConfig object; if there is an error
- *          a \c NULL pointer is returned and \c errno will be set to one of several
- *          potential error values (see defs.h)
+ *          a \c NULL pointer is returned and \c error will be set to a FlogError
+ *          variant representing an error condition
  */
-FlogConfig * flog_config_new(int argc, char *argv[]);
+FlogConfig * flog_config_new(int argc, char *argv[], FlogError *error);
 
 /*! \brief Free a FlogConfig object.
  *

src/flog.c

@@ -22,11 +22,10 @@
 
 #include "flog.h"
 #include <os/log.h>
-#include <sys/errno.h>
 #include <sys/stat.h>
 #include <assert.h>
 #include <stdlib.h>
-#include "defs.h"
+#include "common.h"
 #include "config.h"
 
 #ifdef UNIT_TESTING
@@ -50,12 +49,15 @@ struct FlogCliData {
 };
 
 FlogCli *
-flog_cli_new(FlogConfig *config) {
+flog_cli_new(FlogConfig *config, FlogError *error) {
     assert(config != NULL);
+    assert(error != NULL);
+
+    *error = FLOG_ERROR_NONE;
 
     FlogCli *flog = calloc(1, sizeof(struct FlogCliData));
     if (flog == NULL) {
-        errno = ERR_FLOG_ALLOCATION;
+        *error = FLOG_ERROR_ALLOC;
         return NULL;
     }
 
@@ -109,7 +111,7 @@ flog_commit_message(FlogCli *flog) {
     }
 }
 
-void
+FlogError
 flog_append_message_output(FlogCli *flog) {
     assert(flog != NULL);
 
@@ -122,15 +124,17 @@ flog_append_message_output(FlogCli *flog) {
 
         FILE *fd = fopen(output_file, "a");
         if (fd == NULL) {
-            fprintf(stderr, "%s: unable to append log message to file (%s)\n", PROGRAM_NAME, strerror(errno));
-            exit(errno);
+            umask(original_umask);
+            return FLOG_ERROR_APPEND;
         }
 
         umask(original_umask);
 
         fprintf(fd, "%s", flog_config_get_message(config));
         fclose(fd);
     }
+
+    return FLOG_ERROR_NONE;
 }
 
 void
@@ -142,19 +146,19 @@ flog_commit_public_message(FlogCli *flog) {
     const char *message = flog_config_get_message(config);
 
     switch (level) {
-        case Default:
+        case LVL_DEFAULT:
             os_log(flog->log, OS_LOG_FORMAT_PUBLIC, message);
             break;
-        case Info:
+        case LVL_INFO:
             os_log_info(flog->log, OS_LOG_FORMAT_PUBLIC, message);
             break;
-        case Debug:
+        case LVL_DEBUG:
             os_log_debug(flog->log, OS_LOG_FORMAT_PUBLIC, message);
             break;
-        case Error:
+        case LVL_ERROR:
             os_log_error(flog->log, OS_LOG_FORMAT_PUBLIC, message);
             break;
-        case Fault:
+        case LVL_FAULT:
             os_log_fault(flog->log, OS_LOG_FORMAT_PUBLIC, message);
             break;
         default:
@@ -172,19 +176,19 @@ flog_commit_private_message(FlogCli *flog) {
     const char *message = flog_config_get_message(config);
 
     switch (level) {
-        case Default:
+        case LVL_DEFAULT:
             os_log(flog->log, OS_LOG_FORMAT_PRIVATE, message);
             break;
-        case Info:
+        case LVL_INFO:
             os_log_info(flog->log, OS_LOG_FORMAT_PRIVATE, message);
             break;
-        case Debug:
+        case LVL_DEBUG:
             os_log_debug(flog->log, OS_LOG_FORMAT_PRIVATE, message);
             break;
-        case Error:
+        case LVL_ERROR:
             os_log_error(flog->log, OS_LOG_FORMAT_PRIVATE, message);
             break;
-        case Fault:
+        case LVL_FAULT:
             os_log_fault(flog->log, OS_LOG_FORMAT_PRIVATE, message);
             break;
         default: