From 6f58ef13c44fe9a7e580acd3bd28f92f4208ac59 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 2 Feb 2011 09:38:19 +0100 Subject: [PATCH] Improve error cleanup in output function. Thanks to iswalker. --- src/alert-debuglog.c | 50 +++++++++++++++++++++++++++----------- src/alert-unified-alert.c | 39 ++++++++++++++++++----------- src/alert-unified-log.c | 36 ++++++++++++++++++--------- src/alert-unified2-alert.c | 42 ++++++++++++++++++++------------ 4 files changed, 112 insertions(+), 55 deletions(-) diff --git a/src/alert-debuglog.c b/src/alert-debuglog.c index 7df19f421c..9f802fd5cd 100644 --- a/src/alert-debuglog.c +++ b/src/alert-debuglog.c @@ -420,20 +420,33 @@ void AlertDebugLogExitPrintStats(ThreadVars *tv, void *data) { SCLogInfo("(%s) Alerts %" PRIu64 "", tv->name, aft->file_ctx->alerts); } +static void AlertDebugLogDeInitCtx(OutputCtx *output_ctx) +{ + if (output_ctx != NULL) { + LogFileCtx *logfile_ctx = (LogFileCtx *)output_ctx->data; + if (logfile_ctx != NULL) { + LogFileFreeCtx(logfile_ctx); + } + free(output_ctx); + } +} -/** \brief Create a new LogFileCtx for alert debug logging. +/** + * \brief Create a new LogFileCtx for alert debug logging. + * * \param ConfNode containing configuration for this logger. - * \return NULL if failure, LogFileCtx* to the file_ctx if succesful - * */ + * + * \return output_ctx if succesful, NULL otherwise + */ OutputCtx *AlertDebugLogInitCtx(ConfNode *conf) { - int ret=0; - LogFileCtx* file_ctx=LogFileNewCtx(); + int ret = 0; + LogFileCtx *file_ctx = NULL; - if(file_ctx == NULL) - { - SCLogDebug("AlertDebugLogInitCtx: Couldn't create new file_ctx"); - return NULL; + file_ctx = LogFileNewCtx(); + if(file_ctx == NULL) { + SCLogDebug("couldn't create new file_ctx"); + goto error; } const char *filename = ConfNodeLookupChildValue(conf, "filename"); @@ -445,17 +458,26 @@ OutputCtx *AlertDebugLogInitCtx(ConfNode *conf) mode = DEFAULT_LOG_MODE_APPEND; /** fill the new LogFileCtx with the specific AlertDebugLog configuration */ - ret=AlertDebugLogOpenFileCtx(file_ctx, filename, mode); - + ret = AlertDebugLogOpenFileCtx(file_ctx, filename, mode); if(ret < 0) - return NULL; + goto error; - OutputCtx *output_ctx = SCCalloc(1, sizeof(OutputCtx)); + OutputCtx *output_ctx = SCMalloc(sizeof(OutputCtx)); if (output_ctx == NULL) - return NULL; + goto error; + + memset(output_ctx, 0x00, sizeof(OutputCtx)); output_ctx->data = file_ctx; + output_ctx->DeInit = AlertDebugLogDeInitCtx; return output_ctx; + +error: + if (file_ctx != NULL) { + LogFileFreeCtx(file_ctx); + } + + return NULL; } /** \brief Read the config set the file pointer, open the file diff --git a/src/alert-unified-alert.c b/src/alert-unified-alert.c index b4124a2f78..c5ab1ae171 100644 --- a/src/alert-unified-alert.c +++ b/src/alert-unified-alert.c @@ -299,12 +299,12 @@ error: OutputCtx *AlertUnifiedAlertInitCtx(ConfNode *conf) { int ret = 0; - LogFileCtx *file_ctx = LogFileNewCtx(); + LogFileCtx *file_ctx = NULL; + file_ctx = LogFileNewCtx(); if (file_ctx == NULL) { - SCLogError(SC_ERR_UNIFIED_ALERT_GENERIC, - "AlertUnifiedAlertInitCtx: Couldn't create new file_ctx"); - return NULL; + SCLogError(SC_ERR_UNIFIED_ALERT_GENERIC, "Couldn't create new file_ctx"); + goto error; } const char *filename = NULL; @@ -328,7 +328,7 @@ OutputCtx *AlertUnifiedAlertInitCtx(ConfNode *conf) if (limit < MIN_LIMIT) { SCLogError(SC_ERR_INVALID_ARGUMENT, "Fail to initialize unified alert output, limit less than " - "allowed minimum."); + "allowed minimum: %d.", MIN_LIMIT); exit(EXIT_FAILURE); } } @@ -337,11 +337,11 @@ OutputCtx *AlertUnifiedAlertInitCtx(ConfNode *conf) ret = AlertUnifiedAlertOpenFileCtx(file_ctx, filename); if (ret < 0) - return NULL; + goto error; OutputCtx *output_ctx = SCCalloc(1, sizeof(OutputCtx)); if (output_ctx == NULL) - return NULL; + goto error; output_ctx->data = file_ctx; output_ctx->DeInit = AlertUnifiedAlertDeInitCtx; @@ -349,13 +349,23 @@ OutputCtx *AlertUnifiedAlertInitCtx(ConfNode *conf) filename, limit); return output_ctx; + +error: + if (file_ctx != NULL) { + LogFileFreeCtx(file_ctx); + } + return NULL; } static void AlertUnifiedAlertDeInitCtx(OutputCtx *output_ctx) { - LogFileCtx *logfile_ctx = (LogFileCtx *)output_ctx->data; - LogFileFreeCtx(logfile_ctx); - free(output_ctx); + if (output_ctx != NULL) { + LogFileCtx *logfile_ctx = (LogFileCtx *)output_ctx->data; + if (logfile_ctx != NULL) { + LogFileFreeCtx(logfile_ctx); + } + free(output_ctx); + } } /** \brief Read the config set the file pointer, open the file @@ -392,12 +402,11 @@ int AlertUnifiedAlertOpenFileCtx(LogFileCtx *file_ctx, const char *prefix) snprintf(filename, PATH_MAX, "%s/%s.%" PRIu32, log_dir, prefix, (uint32_t)ts.tv_sec); - /* XXX filename & location */ file_ctx->fp = fopen(filename, "wb"); if (file_ctx->fp == NULL) { SCLogError(SC_ERR_FOPEN, "ERROR: failed to open %s: %s", filename, strerror(errno)); - return TM_ECODE_FAILED; + return -1; } file_ctx->flags = 0; @@ -406,10 +415,12 @@ int AlertUnifiedAlertOpenFileCtx(LogFileCtx *file_ctx, const char *prefix) if (ret != 0) { SCLogError(SC_ERR_UNIFIED_ALERT_GENERIC, "Error: AlertUnifiedLogWriteFileHeader failed"); - return TM_ECODE_FAILED; + + fclose(file_ctx->fp); + return -1; } - return TM_ECODE_OK; + return 0; } #ifdef UNITTESTS diff --git a/src/alert-unified-log.c b/src/alert-unified-log.c index d3f125bfa4..a0d4681b25 100644 --- a/src/alert-unified-log.c +++ b/src/alert-unified-log.c @@ -341,11 +341,12 @@ error: OutputCtx *AlertUnifiedLogInitCtx(ConfNode *conf) { int ret = 0; - LogFileCtx* file_ctx=LogFileNewCtx(); + LogFileCtx* file_ctx = NULL; + file_ctx = LogFileNewCtx(); if (file_ctx == NULL) { SCLogError(SC_ERR_MEM_ALLOC, "Couldn't create new file_ctx"); - return NULL; + goto error; } const char *filename = NULL; @@ -370,7 +371,7 @@ OutputCtx *AlertUnifiedLogInitCtx(ConfNode *conf) if (limit < MIN_LIMIT) { SCLogError(SC_ERR_INVALID_ARGUMENT, "Fail to initialize unified log output, limit less than " - "allowed minimum."); + "allowed minimum: %d.", MIN_LIMIT); exit(EXIT_FAILURE); } SCLogDebug("limit set to %"PRIu32, limit); @@ -380,11 +381,11 @@ OutputCtx *AlertUnifiedLogInitCtx(ConfNode *conf) ret = AlertUnifiedLogOpenFileCtx(file_ctx, filename); if (ret < 0) - return NULL; + goto error; OutputCtx *output_ctx = SCCalloc(1, sizeof(OutputCtx)); if (output_ctx == NULL) - return NULL; + goto error; output_ctx->data = file_ctx; output_ctx->DeInit = AlertUnifiedLogDeInitCtx; @@ -392,13 +393,23 @@ OutputCtx *AlertUnifiedLogInitCtx(ConfNode *conf) filename, limit); return output_ctx; + +error: + if (file_ctx != NULL) { + LogFileFreeCtx(file_ctx); + } + return NULL; } static void AlertUnifiedLogDeInitCtx(OutputCtx *output_ctx) { - LogFileCtx *logfile_ctx = (LogFileCtx *)output_ctx->data; - LogFileFreeCtx(logfile_ctx); - free(output_ctx); + if (output_ctx != NULL) { + LogFileCtx *logfile_ctx = (LogFileCtx *)output_ctx->data; + if (logfile_ctx != NULL) { + LogFileFreeCtx(logfile_ctx); + } + free(output_ctx); + } } /** \brief Read the config set the file pointer, open the file @@ -435,22 +446,23 @@ int AlertUnifiedLogOpenFileCtx(LogFileCtx *file_ctx, const char *prefix) snprintf(filename, PATH_MAX, "%s/%s.%" PRIu32, log_dir, prefix, (uint32_t)ts.tv_sec); - /* XXX filename & location */ file_ctx->fp = fopen(filename, "wb"); if (file_ctx->fp == NULL) { SCLogError(SC_ERR_FOPEN, "ERROR: failed to open %s: %s", filename, strerror(errno)); - return TM_ECODE_FAILED; + return -1; } /** Write Unified header */ ret = AlertUnifiedLogWriteFileHeader(file_ctx); if (ret != 0) { printf("Error: AlertUnifiedLogWriteFileHeader failed.\n"); - return TM_ECODE_FAILED; + + fclose(file_ctx->fp); + return -1; } - return TM_ECODE_OK; + return 0; } #ifdef UNITTESTS diff --git a/src/alert-unified2-alert.c b/src/alert-unified2-alert.c index e1281270e9..2d97f2e945 100644 --- a/src/alert-unified2-alert.c +++ b/src/alert-unified2-alert.c @@ -621,13 +621,13 @@ error: * */ OutputCtx *Unified2AlertInitCtx(ConfNode *conf) { - int ret=0; - LogFileCtx* file_ctx=LogFileNewCtx(); + int ret = 0; + LogFileCtx* file_ctx = NULL; + file_ctx = LogFileNewCtx(); if (file_ctx == NULL) { - SCLogError(SC_ERR_UNIFIED2_ALERT_GENERIC, "Unified2AlertInitCtx: " - "Couldn't create new file_ctx"); - return NULL; + SCLogError(SC_ERR_UNIFIED2_ALERT_GENERIC, "Couldn't create new file_ctx"); + goto error; } const char *filename = NULL; @@ -645,14 +645,14 @@ OutputCtx *Unified2AlertInitCtx(ConfNode *conf) if (s_limit != NULL) { if (ByteExtractStringUint32(&limit, 10, 0, s_limit) == -1) { SCLogError(SC_ERR_INVALID_ARGUMENT, - "Fail to initialize unified2 output, invalid limit: %s", + "Failed to initialize unified2 output, invalid limit: %s", s_limit); exit(EXIT_FAILURE); } if (limit < MIN_LIMIT) { SCLogError(SC_ERR_INVALID_ARGUMENT, - "Fail to initialize unified2 output, limit less than " - "allowed minimum."); + "Failed to initialize unified2 output, limit less than " + "allowed minimum: %d.", MIN_LIMIT); exit(EXIT_FAILURE); } } @@ -661,11 +661,11 @@ OutputCtx *Unified2AlertInitCtx(ConfNode *conf) ret = Unified2AlertOpenFileCtx(file_ctx, filename); if (ret < 0) - return NULL; + goto error; OutputCtx *output_ctx = SCCalloc(1, sizeof(OutputCtx)); if (output_ctx == NULL) - return NULL; + goto error; output_ctx->data = file_ctx; output_ctx->DeInit = Unified2AlertDeInitCtx; @@ -673,13 +673,24 @@ OutputCtx *Unified2AlertInitCtx(ConfNode *conf) filename, limit); return output_ctx; + +error: + if (file_ctx != NULL) { + LogFileFreeCtx(file_ctx); + } + + return NULL; } static void Unified2AlertDeInitCtx(OutputCtx *output_ctx) { - LogFileCtx *logfile_ctx = (LogFileCtx *)output_ctx->data; - LogFileFreeCtx(logfile_ctx); - free(output_ctx); + if (output_ctx != NULL) { + LogFileCtx *logfile_ctx = (LogFileCtx *)output_ctx->data; + if (logfile_ctx != NULL) { + LogFileFreeCtx(logfile_ctx); + } + free(output_ctx); + } } /** \brief Read the config set the file pointer, open the file @@ -697,6 +708,8 @@ int Unified2AlertOpenFileCtx(LogFileCtx *file_ctx, const char *prefix) filename = file_ctx->filename = SCMalloc(PATH_MAX); /* XXX some sane default? */ if (filename == NULL) return -1; + + memset(filename, 0x00, PATH_MAX); } /** get the time so we can have a filename with seconds since epoch */ @@ -716,10 +729,9 @@ int Unified2AlertOpenFileCtx(LogFileCtx *file_ctx, const char *prefix) snprintf(filename, PATH_MAX, "%s/%s.%" PRIu32, log_dir, prefix, (uint32_t)ts.tv_sec); - /* XXX filename & location */ file_ctx->fp = fopen(filename, "wb"); if (file_ctx->fp == NULL) { - SCLogError(SC_ERR_FOPEN, "ERROR: failed to open %s: %s", filename, + SCLogError(SC_ERR_FOPEN, "failed to open %s: %s", filename, strerror(errno)); ret = -1; }