implemented logging

added better error messages for DB connections
fix several segfaults

8 files changed, 132 insertions, 77 deletions

diff --git a/utility/config.c b/utility/config.c
index ccdc6e5..b1e7585 100644
--- a/utility/config.c
+++ b/utility/config.c
@@ -41,12 +41,10 @@ static apr_status_t config_set_loglevel(config_t *cfg, config_opt_t *opt,
         return APR_EINVAL;
     if (!strcasecmp(argv[1], "error")) {
         cfg->loglevel = LOGLEVEL_ERROR;
-    } else if (!strcasecmp(argv[1], "warn")) {
-        cfg->loglevel = LOGLEVEL_WARN;
+    } else if (!strcasecmp(argv[1], "notice")) {
+        cfg->loglevel = LOGLEVEL_NOTICE;
     } else if (!strcasecmp(argv[1], "debug")) {
         cfg->loglevel = LOGLEVEL_DEBUG;
-    } else if (!strcasecmp(argv[1], "quiet")) {
-        cfg->loglevel = LOGLEVEL_QUIET;
     } else {
         cfg->loglevel = LOGLEVEL_ERROR;
     }
@@ -293,6 +291,8 @@ void config_init(apr_pool_t *p)
 
     config_add_option(p, "DryRun", "Don't perform any actual database changes",
             config_set_flag, (void *)APR_OFFSETOF(config_t, dryrun));
+    config_add_option(p, "Dump", "Dump Configuration and quit",
+            config_set_flag, (void *)APR_OFFSETOF(config_t, dump));
     config_add_option(p, "Config", "Dummy to handle config directive",
             config_set_dummy, NULL);
     config_add_option(p, "Summary", "Show the summary before exit?",
@@ -306,7 +306,7 @@ config_t *config_create(apr_pool_t *p)
     apr_pool_create(&sp, p);
     cfg = apr_pcalloc(sp, sizeof(config_t));
     cfg->pool = sp;
-    cfg->loglevel = LOGLEVEL_WARN;
+    cfg->loglevel = LOGLEVEL_ERROR;
     cfg->summary = 1;
     cfg->transactions = 1;
     cfg->input_files = apr_array_make(cfg->pool, 10, sizeof(char *));
@@ -320,15 +320,15 @@ apr_status_t config_check(config_t *cfg)
 {
     apr_status_t ret = APR_SUCCESS;
     if (!cfg->dbdriver || !cfg->dbparams) {
-        printf("Database configuration is missing\n");
+        logging_log(cfg, LOGLEVEL_NOISE, "Database configuration is missing\n");
         ret = APR_EINVAL;
     }
     if (!cfg->table) {
-        printf("No Log Table defined\n");
+        logging_log(cfg, LOGLEVEL_NOISE, "No Log Table defined\n");
         ret = APR_EINVAL;
     }
     if (apr_is_empty_array(cfg->output_fields)) {
-        printf("No Output Fields Defined\n");
+        logging_log(cfg, LOGLEVEL_NOISE, "No Output Fields Defined\n");
         ret = APR_EINVAL;
     }
     return ret;
@@ -345,7 +345,7 @@ static int config_merge(void *rec, const char *key, const char *value)
             value };
         opt->func(cfg, opt, 2, args);
     } else {
-        printf("Unhandled: %s\n", key);
+        logging_log(cfg, LOGLEVEL_NOISE, "Unhandled: %s\n", key);
     }
     return 1;
 }
@@ -400,12 +400,13 @@ apr_status_t config_read(config_t *cfg, const char *filename,
             if (opt) {
                 rv = opt->func(cfg, opt, targc, (const char **)targv);
                 if (APR_STATUS_IS_EINVAL(rv)) {
-                    printf("Config Error: Invalid Arguments for %s\n\t%s\n",
+                    logging_log(cfg, LOGLEVEL_NOISE,
+                            "Config Error: Invalid Arguments for %s\n\t%s\n",
                             opt->name, opt->help);
                     ret = rv;
                 }
             } else {
-                printf("Unhandled: %s\n", targv[0]);
+                logging_log(cfg, LOGLEVEL_NOISE, "Unhandled: %s\n", targv[0]);
             }
         }
     } while (rv == APR_SUCCESS);
diff --git a/utility/config.h b/utility/config.h
index 611e9de..763ef5d 100644
--- a/utility/config.h
+++ b/utility/config.h
@@ -7,10 +7,11 @@
 #include "ap_pcre.h"
 
 typedef enum {
-    LOGLEVEL_QUIET = 0,
-    LOGLEVEL_ERROR = 1,
-    LOGLEVEL_WARN = 2,
-    LOGLEVEL_DEBUG = 3,
+    LOGLEVEL_NOISE = 0,
+    LOGLEVEL_NONE,
+    LOGLEVEL_ERROR,
+    LOGLEVEL_NOTICE,
+    LOGLEVEL_DEBUG,
 } loglevel_e;
 
 typedef struct config_dbd_t config_dbd_t;
@@ -25,6 +26,7 @@ struct config_t {
     loglevel_e loglevel;
     /** error_log */
     apr_file_t *errorlog_fp;
+    apr_file_t *errorlog_fperr;
     apr_pool_t *errorlog_p;
 
     /** input directory of log files */
@@ -59,6 +61,8 @@ struct config_t {
 
     /** Dry Run */
     int dryrun;
+    /** dump configuration only */
+    int dump;
 
     /* Show the summary */
     int summary;
diff --git a/utility/database.c b/utility/database.c
index 45022da..c4e4bc9 100644
--- a/utility/database.c
+++ b/utility/database.c
@@ -4,6 +4,7 @@
 #include "apr_strings.h"
 
 #include "util.h"
+#include "mysql/mysql.h"
 
 struct config_dbd_t {
     const apr_dbd_driver_t *driver;
@@ -27,9 +28,10 @@ apr_status_t database_connect(config_t *cfg)
     }
     rv = apr_dbd_get_driver(cfg->pool, cfg->dbdriver, &(cfg->dbconn->driver));
     if (rv) {
+
         logging_log(cfg, LOGLEVEL_ERROR,
-                "Could not load database driver %s. Error %d", cfg->dbdriver,
-                rv);
+                "Could not load database driver %s. Error %s", cfg->dbdriver,
+                logging_strerror(rv));
         return rv;
     }
 
@@ -37,7 +39,7 @@ apr_status_t database_connect(config_t *cfg)
             &(cfg->dbconn->dbd));
     if (rv) {
         logging_log(cfg, LOGLEVEL_ERROR,
-                "Could not connect to database. Error %d", rv);
+                "Could not connect to database. Error %s", logging_strerror(rv));
         return rv;
     }
 
@@ -55,7 +57,7 @@ static apr_dbd_prepared_t *database_prepare_insert(config_t *cfg, apr_pool_t *p)
     char *sql;
     int i, f;
     struct iovec *vec;
-    apr_dbd_prepared_t *stmt;
+    apr_dbd_prepared_t *stmt = NULL;
     int nfs = cfg->output_fields->nelts;
     config_output_field_t *ofields;
 
@@ -87,14 +89,14 @@ static apr_dbd_prepared_t *database_prepare_insert(config_t *cfg, apr_pool_t *p)
 
     sql = apr_pstrcatv(p, vec, i+2, NULL);
 
-    printf("SQL: %s\n", sql);
+    logging_log(cfg, LOGLEVEL_DEBUG, "Generated SQL: %s", sql);
 
     rv = apr_dbd_prepare(cfg->dbconn->driver, cfg->pool, cfg->dbconn->dbd, sql,
             "INSERT", &stmt);
 
     if (rv) {
-        printf("DB Error: %s\n", apr_dbd_error(cfg->dbconn->driver,
-                cfg->dbconn->dbd, rv));
+        logging_log(cfg, LOGLEVEL_ERROR, "Unable to Prepare SQL insert: %s",
+                apr_dbd_error(cfg->dbconn->driver, cfg->dbconn->dbd, rv));
         return NULL;
     }
     return stmt;
@@ -109,6 +111,10 @@ apr_status_t database_insert(config_t *cfg, apr_pool_t *p, apr_table_t *data)
     // Prepare statement
     if (!cfg->dbconn->stmt) {
         cfg->dbconn->stmt = database_prepare_insert(cfg, p);
+        if (!cfg->dbconn->stmt) {
+            logging_log(cfg, LOGLEVEL_NOISE, "Unable to prepare SQL statement");
+            return APR_EINVAL;
+        }
         cfg->dbconn->args = apr_palloc(cfg->pool, nfs * sizeof(char *));
     }
     for (f=0; f<nfs; f++) {
@@ -117,8 +123,8 @@ apr_status_t database_insert(config_t *cfg, apr_pool_t *p, apr_table_t *data)
     rv = apr_dbd_pquery(cfg->dbconn->driver, p, cfg->dbconn->dbd, &f,
             cfg->dbconn->stmt, nfs, cfg->dbconn->args);
     if (rv) {
-        printf("DB Error: %s\n", apr_dbd_error(cfg->dbconn->driver,
-                cfg->dbconn->dbd, rv));
+        logging_log(cfg, LOGLEVEL_ERROR, "Unable to Insert SQL: %s",
+                apr_dbd_error(cfg->dbconn->driver, cfg->dbconn->dbd, rv));
         return rv;
     }
     return APR_SUCCESS;
diff --git a/utility/logparse.c b/utility/logparse.c
index 4b5ab40..f4afb52 100644
--- a/utility/logparse.c
+++ b/utility/logparse.c
@@ -11,7 +11,6 @@
 #include "ap_pcre.h"
 #include "database.h"
 
-
 apr_hash_t *g_parser_funcs;
 
 static apr_status_t parser_func_regexmatch(apr_pool_t *p, config_t *cfg,
@@ -20,23 +19,26 @@ static apr_status_t parser_func_regexmatch(apr_pool_t *p, config_t *cfg,
     struct {
         ap_regex_t *rx;
         const char *substr;
-    } *data;
+    }*data;
     ap_regmatch_t regm[AP_MAX_REG_MATCH];
     // Check if a regular expression configured
-    if (!field->args[0]) return APR_EINVAL;
+    if (!field->args[0])
+        return APR_EINVAL;
     if (!field->data) {
         // pre compile the regex
         data = apr_palloc(cfg->pool, sizeof(ap_regex_t)+sizeof(const char *));
         data->rx = ap_pregcomp(cfg->pool, field->args[0],
-                AP_REG_EXTENDED|AP_REG_ICASE);
+        AP_REG_EXTENDED|AP_REG_ICASE);
         if (field->args[1]) {
             data->substr = field->args[1];
         } else {
             data->substr = "$1";
         }
-        if (!data->rx) return APR_EINVAL;
+        if (!data->rx)
+            return APR_EINVAL;
         field->data = data;
-    } else data = field->data;
+    } else
+        data = field->data;
 
     if (!ap_regexec(data->rx, value, AP_MAX_REG_MATCH, regm, 0)) {
         *ret = ap_pregsub(p, data->substr, value, AP_MAX_REG_MATCH, regm);
@@ -66,7 +68,7 @@ static apr_status_t parser_func_machineid(apr_pool_t *p, config_t *cfg,
         config_output_field_t *field, const char *value, const char **ret)
 {
     if (cfg->machineid) {
-        *ret = apr_pstrdup(p,cfg->machineid);
+        *ret = apr_pstrdup(p, cfg->machineid);
     } else {
         *ret = field->def;
     }
@@ -81,7 +83,7 @@ parser_func_t parser_get_func(const char *name)
 }
 
 static void parser_add_func(apr_pool_t *p, const char *const name,
-           parser_func_t func)
+        parser_func_t func)
 {
     if (!g_parser_funcs) {
         g_parser_funcs = apr_hash_make(p);
@@ -103,6 +105,7 @@ void parser_find_logs(config_t *cfg)
     apr_finfo_t finfo;
     char **newp;
 
+    logging_log(cfg, LOGLEVEL_NOTICE, "Find Log files");
     if (!cfg->input_dir)
         return;
     apr_pool_create(&tp, cfg->pool);
@@ -113,7 +116,7 @@ void parser_find_logs(config_t *cfg)
                 continue;
             newp = (char **)apr_array_push(cfg->input_files);
             apr_filepath_merge(newp, cfg->input_dir, finfo.name,
-                    APR_FILEPATH_TRUENAME, cfg->pool);
+            APR_FILEPATH_TRUENAME, cfg->pool);
         }
         apr_dir_close(dir);
     }
@@ -261,11 +264,12 @@ apr_status_t parse_logfile(config_t *cfg, const char *filename)
     apr_pool_create(&tp, cfg->pool);
     apr_pool_create(&targp, tp);
 
+    logging_log(cfg, LOGLEVEL_NOTICE, "Begin Parsing Log File '%s'", filename);
+
     rv = apr_file_open(&file, filename, APR_FOPEN_READ | APR_BUFFERED,
-            APR_OS_DEFAULT, tp);
+    APR_OS_DEFAULT, tp);
     if (rv != APR_SUCCESS) {
-        logging_log(cfg, LOGLEVEL_ERROR, "Could not open %s",filename);
-        printf("Could not open %s\n", filename);
+        logging_log(cfg, LOGLEVEL_NOISE, "Could not open %s", filename);
         return rv;
     }
 
@@ -280,44 +284,52 @@ apr_status_t parse_logfile(config_t *cfg, const char *filename)
             apr_pool_clear(targp);
             tokenize_logline(buff, &targv, targp, 0);
             targc = 0;
-            while (targv[targc]) targc++;
+            while (targv[targc])
+                targc++;
             /** @todo Run Line Filters here */
             rv = parse_processline(targp, cfg, targv, targc);
             if (rv != APR_SUCCESS) {
                 int i;
-                printf("Line %d(%d): %s\n",line, targc, buff);
+                logging_log(cfg, LOGLEVEL_ERROR, "Line %d(%d): %s", line,
+                        targc, buff);
                 for (i = 0; targv[i]; i++) {
-                    printf("Arg (%d): '%s'\n", i, targv[i]);
+                    logging_log(cfg, LOGLEVEL_ERROR, "Arg (%d): '%s'", i,
+                            targv[i]);
                 }
             }
         }
     } while (rv == APR_SUCCESS);
-    printf("Total Lines: %d\n", line);
     apr_file_close(file);
     apr_pool_destroy(tp);
+    logging_log(cfg, LOGLEVEL_NOTICE,
+            "Finish Parsing Log File '%s'. Lines: %d", filename, line);
+
     return APR_SUCCESS;
 }
 
-apr_status_t parse_processline(apr_pool_t *ptemp, config_t *cfg, char **argv, int argc)
+apr_status_t parse_processline(apr_pool_t *ptemp, config_t *cfg, char **argv,
+        int argc)
 {
     config_logformat_t *fmt;
     config_logformat_field_t *ifields;
     config_output_field_t *ofields;
     apr_table_t *datain;
     apr_table_t *dataout;
-    apr_status_t rv = APR_SUCCESS;
+    apr_status_t rv= APR_SUCCESS;
     int i;
 
     fmt = apr_hash_get(cfg->log_formats, cfg->logformat, APR_HASH_KEY_STRING);
-    if (!fmt) return APR_EINVAL;
-    if (fmt->fields->nelts != argc) return APR_EINVAL;
+    if (!fmt)
+        return APR_EINVAL;
+    if (fmt->fields->nelts != argc)
+        return APR_EINVAL;
 
     datain = apr_table_make(ptemp, fmt->fields->nelts);
     dataout = apr_table_make(ptemp, cfg->output_fields->nelts);
 
     ifields = (config_logformat_field_t *)fmt->fields->elts;
     for (i=0; i<fmt->fields->nelts; i++) {
-        apr_table_setn(datain,ifields[i].name,argv[i]);
+        apr_table_setn(datain, ifields[i].name, argv[i]);
     }
     /** @todo Run Pre Filters here */
 
@@ -332,12 +344,13 @@ apr_status_t parse_processline(apr_pool_t *ptemp, config_t *cfg, char **argv, in
             continue;
         }
         if (!ofields[i].func) {
-            apr_table_setn(dataout,ofields[i].field, val);
+            apr_table_setn(dataout, ofields[i].field, val);
         } else {
-            const char *ret = NULL;
+            const char *ret= NULL;
             rv = ((parser_func_t)ofields[i].func)(ptemp, cfg, &ofields[i], val,
                     &ret);
-            if (rv) return rv;
+            if (rv)
+                return rv;
             apr_table_setn(dataout, ofields[i].field, ret);
         }
     }
diff --git a/utility/mod_log_sql.conf b/utility/mod_log_sql.conf
index bd8496c..771f7c7 100644
--- a/utility/mod_log_sql.conf
+++ b/utility/mod_log_sql.conf
@@ -5,8 +5,8 @@ DBDParams "host=localhost;user=root;dbname=apache_log"
 Table access_log
 MachineID 7of9
 UseTransactions on
-LogLevel notice
-DryRun on
+LogLevel debug
+DryRun off
 Summary on
 
 LogFormatConfig CLF remhost String
diff --git a/utility/shell.c b/utility/shell.c
index b289bce..e521916 100644
--- a/utility/shell.c
+++ b/utility/shell.c
@@ -17,9 +17,10 @@ const apr_getopt_option_t _opt_config[]   = {
     {"transaction", 't',    1,  "Use a Transaction (yes,no)"},
     {"logformat",   'r',    1,  "Use this logformat to parse files"},
     {"file",        'f',    1,  "Parse this single log file (input dir is NOT scanned)"},
-    {"inputdir",    'd',    1,  "Input Directory to look for log files"},
+    {"inputdir",    'i',    1,  "Input Directory to look for log files"},
     {"config",      'c',    1,  "Configuration file to use (default mod_log_sql.conf)"},
     {"dryrun",      'n',    0,  "Perform a dry run (do not actually alter the databse)"},
+    {"dump",        'd',    0,  "Dump the configuration after parsing and quit"},
     {"loglevel",    'l',    1,  "Log Level (deubg, warn, error)"},
     {"summary",     's',    1,  "Summary (yes,no)"},
     {"help",        'h',    0,  "Show Help"},
@@ -62,7 +63,7 @@ int main(int argc, const char *const argv[])
     const char *opt_arg;
     apr_status_t rv;
     apr_table_t *args;
-    config_t *base;
+    config_t *cfg;
 
     apr_app_initialize(&argc, &argv, NULL);
     atexit(apr_terminate);
@@ -84,7 +85,7 @@ int main(int argc, const char *const argv[])
             apr_table_setn(args,"config",opt_arg);
             break;
         case 'd':
-            apr_table_setn(args,"inputdirectory",opt_arg);
+            apr_table_setn(args,"dump","yes");
             break;
         case 'f':
             apr_table_setn(args,"inputfile",opt_arg);
@@ -93,6 +94,9 @@ int main(int argc, const char *const argv[])
             show_help(argv[0], _opt_config, stdout);
             exit(1);
             break;
+        case 'i':
+            apr_table_setn(args,"inputdirectory",opt_arg);
+            break;
         case 'l':
             apr_table_setn(args,"loglevel",opt_arg);
             break;
@@ -128,45 +132,49 @@ int main(int argc, const char *const argv[])
     parser_init(pool);
     config_init(pool);
     database_init(pool);
-    logging_init(pool);
     // Process configuration file
-    base = config_create(pool);
-    rv = config_read(base, apr_table_get(args,"Config"), args);
+    cfg = config_create(pool);
+    rv = config_read(cfg, apr_table_get(args,"Config"), args);
     apr_pool_destroy(ptemp);
 
+    // Initialize Log system AFTER we parse the configuration
+    logging_init(cfg);
+
     if (APR_STATUS_IS_ENOENT(rv)) {
-        fprintf(stderr,"Could not load configuration file: %s\n",apr_table_get(args,"config"));
+        logging_log(cfg,LOGLEVEL_NOISE,"Could not load configuration file: %s",apr_table_get(args,"config"));
     } else if (rv) {
         exit(1);
     }
-    config_dump(base);
+    if (cfg->dump) {
+        config_dump(cfg);
+        exit(0);
+    }
 
-    if (config_check(base)) {
-        printf("Please correct the configuration\n");
+    if (config_check(cfg)) {
+        logging_log(cfg,LOGLEVEL_NOISE, "Please correct the configuration");
         exit(1);
     }
 
     // Find files and parse
-    parser_find_logs(base);
-    if (!base->dryrun) {
-        if ((rv = database_connect(base))) {
-            printf("Error Connecting to Database: %d\n",rv);
+    parser_find_logs(cfg);
+    if (!cfg->dryrun) {
+        if ((rv = database_connect(cfg))) {
+            logging_log(cfg,LOGLEVEL_NOISE, "Error Connecting to Database");
             exit(1);
         }
     }
-    if (!apr_is_empty_array(base->input_files)) {
+    if (!apr_is_empty_array(cfg->input_files)) {
         char **filelist;
         int f, l;
-        filelist = (char **)base->input_files->elts;
-        for (f=0, l=base->input_files->nelts; f < l; f++) {
-            printf("Scanning %s\n",filelist[f]);
-            parse_logfile(base, filelist[f]);
+        filelist = (char **)cfg->input_files->elts;
+        for (f=0, l=cfg->input_files->nelts; f < l; f++) {
+            parse_logfile(cfg, filelist[f]);
         }
     } else {
-        printf("No input files\n");
+        logging_log(cfg,LOGLEVEL_NOISE,"No log files found to parse");
     }
-    if (!base->dryrun) {
-        database_disconnect(base);
+    if (!cfg->dryrun) {
+        database_disconnect(cfg);
     }
     /** @todo summary goes here */
     return 0;
diff --git a/utility/util.c b/utility/util.c
index cb6898d..99bb046 100644
--- a/utility/util.c
+++ b/utility/util.c
@@ -33,15 +33,31 @@ void line_chomp(char *str)
 
 void logging_init(config_t *cfg)
 {
+    apr_status_t rv;
+    apr_pool_create(&cfg->errorlog_p, cfg->pool);
+    apr_file_open_stderr(&cfg->errorlog_fperr, cfg->pool);
     if (cfg->errorlog) {
-        apr_file_open(&cfg->errorlog_fp, cfg->errorlog,
-                APR_FOPEN_CREATE | APR_FOPEN_WRITE | APR_BUFFERED,
+        rv = apr_file_open(&cfg->errorlog_fp, cfg->errorlog,
+                APR_FOPEN_CREATE | APR_FOPEN_WRITE | APR_FOPEN_APPEND,
                 APR_OS_DEFAULT,
                 cfg->pool);
-        apr_pool_create(&cfg->errorlog_p, cfg->pool);
+        if (rv) {
+            printf("Error opening %s\n",cfg->errorlog);
+            cfg->loglevel = LOGLEVEL_NONE;
+        }
+        logging_log(cfg, LOGLEVEL_ERROR, "Log file Opened");
+    } else {
+        cfg->loglevel = LOGLEVEL_NONE;
+        logging_log(cfg, LOGLEVEL_NOISE, "No Log file specified, disabled logging");
     }
 }
 
+const char *logging_strerror(apr_status_t rv)
+{
+    char buff[256];
+    return apr_strerror(rv, buff, 256);
+}
+
 /**
  * @todo implement logging
  */
@@ -51,7 +67,7 @@ void logging_log(config_t *cfg, loglevel_e level, const char *fmt, ...)
     struct iovec vec[2];
     apr_size_t blen;
 
-    if (!cfg->errorlog_fp || cfg->loglevel < level) return;
+    if (cfg->loglevel < level) return;
 
     va_start(ap, fmt);
     apr_pool_clear(cfg->errorlog_p);
@@ -61,7 +77,12 @@ void logging_log(config_t *cfg, loglevel_e level, const char *fmt, ...)
     vec[1].iov_base = "\n";
     vec[1].iov_len = 1;
 
-    apr_file_writev(cfg->errorlog_fp,vec,2,&blen);
+    if (level == LOGLEVEL_NOISE) {
+        apr_file_writev(cfg->errorlog_fperr,vec,2,&blen);
+    }
+    if (cfg->loglevel > LOGLEVEL_NONE) {
+        apr_file_writev(cfg->errorlog_fp,vec,2,&blen);
+    }
 
     va_end(ap);
 }
diff --git a/utility/util.h b/utility/util.h
index 9e3aed3..c67cf9c 100644
--- a/utility/util.h
+++ b/utility/util.h
@@ -17,4 +17,6 @@ void logging_init(config_t *cfg);
 void logging_log(config_t *cfg, loglevel_e level, const char *fmt, ...)
             __attribute__((format(printf, 3, 4)));
 
+const char *logging_strerror(apr_status_t rv);
+
 #endif /*UTIL_H_*/