Hi,
This patch corrects (hopefully last) set of issues found in the earlier
patches during review.
The ones that are not addressed yet are tracked via explicit tickets or
via FIXME
comments in the code.
The whole stack of the pending patches is:
* ELAPI sinks and providers
* ELAPI Adding file provider and CSV format
* ELAPI Laying foundation for the async processing
* COLLECTION Copy collection flat with concatenated names
* ELAPI Correcting issues
--
Thank you,
Dmitri Pal
Engineering Manager IPA project,
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
>From 27dd81c86a5e3af703876ac7870257df32769c5b Mon Sep 17 00:00:00 2001
From: Dmitri Pal <d...@redhat.com>
Date: Tue, 8 Sep 2009 18:37:44 -0400
Subject: [PATCH] ELAPI Correcting issues
This patch addresses the issues found during the
review of the previous patches and addresses
ticket #166.
---
common/elapi/configure.ac | 2 +
common/elapi/def_macros.m4 | 5 +
common/elapi/elapi_basic.c | 19 +++--
common/elapi/elapi_basic.h | 15 ++--
common/elapi/elapi_test/configure.ac | 1 +
common/elapi/providers/file/file_fmt_csv.c | 108 ++++++++++++++-------------
common/elapi/providers/file/file_fmt_csv.h | 9 ++-
common/elapi/providers/file/file_provider.c | 14 ++--
common/elapi/providers/file/file_util.c | 14 ++--
common/elapi/providers/file/file_util.h | 2 +-
10 files changed, 105 insertions(+), 84 deletions(-)
create mode 100644 common/elapi/def_macros.m4
diff --git a/common/elapi/configure.ac b/common/elapi/configure.ac
index 010244c..ad2ffce 100644
--- a/common/elapi/configure.ac
+++ b/common/elapi/configure.ac
@@ -28,6 +28,8 @@ WITH_CONFIG_APP_DIR
WITH_APP_NAME
WITH_APP_NAME_SIZE
+m4_include(def_macros.m4)
+
AC_CONFIG_SUBDIRS([elapi_test])
AC_CONFIG_FILES([Makefile elapi.pc])
diff --git a/common/elapi/def_macros.m4 b/common/elapi/def_macros.m4
new file mode 100644
index 0000000..eca94dd
--- /dev/null
+++ b/common/elapi/def_macros.m4
@@ -0,0 +1,5 @@
+# Common defines for ELAPI and its unit test
+
+AC_DEFINE([MAX_LONG_STRING_LEN], [20], [Max length of the serialized long value])
+AC_DEFINE([MAX_DOUBLE_STRING_LEN], [22], [Max length of the serialized double value])
+AC_DEFINE([MAX_BOOL_STRING_LEN], [5], [Max length of the serialized bool value])
diff --git a/common/elapi/elapi_basic.c b/common/elapi/elapi_basic.c
index f528023..8c7ddb7 100644
--- a/common/elapi/elapi_basic.c
+++ b/common/elapi/elapi_basic.c
@@ -27,7 +27,7 @@
#include "config.h"
/* Function to free serialized data */
-void elapi_free_serialized_data(SEDO *out_data)
+void elapi_free_serialized_data(struct elapi_data_out *out_data)
{
TRACE_FLOW_STRING("elapi_free_serialized_data", "Entry");
@@ -40,7 +40,7 @@ void elapi_free_serialized_data(SEDO *out_data)
}
/* Allocate data structure */
-int elapi_alloc_serialized_data(SEDO **out_data)
+int elapi_alloc_serialized_data(struct elapi_data_out **out_data)
{
int error;
@@ -51,7 +51,8 @@ int elapi_alloc_serialized_data(SEDO **out_data)
error = EINVAL;
}
else {
- *out_data = (SEDO *)calloc(1, sizeof(SEDO));
+ *out_data = (struct elapi_data_out *)calloc(1,
+ sizeof(struct elapi_data_out));
if (*out_data == NULL) {
TRACE_ERROR_STRING("Failed to allocate memory", "");
error = ENOMEM;
@@ -65,10 +66,12 @@ int elapi_alloc_serialized_data(SEDO **out_data)
/* Grow buffer */
-int elapi_grow_data(SEDO *out_data, uint32_t len, uint32_t block)
+int elapi_grow_data(struct elapi_data_out *out_data,
+ uint32_t len,
+ uint32_t block)
{
int error = EOK;
- void *tmp = NULL;
+ unsigned char *newbuf = NULL;
TRACE_FLOW_STRING("elapi_grow_data", "Entry");
@@ -79,12 +82,12 @@ int elapi_grow_data(SEDO *out_data, uint32_t len, uint32_t block)
/* Grow buffer if needed */
while (out_data->length + len >= out_data->size) {
- tmp = realloc(out_data->buffer, out_data->size + block);
- if (tmp == NULL) {
+ newbuf = realloc(out_data->buffer, out_data->size + block);
+ if (newbuf == NULL) {
TRACE_ERROR_NUMBER("Error. Failed to allocate memory.", ENOMEM);
return ENOMEM;
}
- out_data->buffer = tmp;
+ out_data->buffer = newbuf;
out_data->size += block;
TRACE_INFO_NUMBER("New size: ", out_data->size);
}
diff --git a/common/elapi/elapi_basic.h b/common/elapi/elapi_basic.h
index 45ff194..8d23c7d 100644
--- a/common/elapi/elapi_basic.h
+++ b/common/elapi/elapi_basic.h
@@ -28,24 +28,21 @@
/* Generic data structure for the data output */
struct elapi_data_out {
- char *buffer;
+ unsigned char *buffer;
uint32_t size;
uint32_t length;
uint32_t written;
};
-/* For simplicity the SEDO stands for
- * struct-elapi-data-out
- */
-#define SEDO struct elapi_data_out
-
/* Function to free serialized data */
-void elapi_free_serialized_data(SEDO *out_data);
+void elapi_free_serialized_data(struct elapi_data_out *out_data);
/* Allocate data structure */
-int elapi_alloc_serialized_data(SEDO **out_data);
+int elapi_alloc_serialized_data(struct elapi_data_out **out_data);
/* Function to add memory to the output buffer */
-int elapi_grow_data(SEDO *out_data, uint32_t len, uint32_t block);
+int elapi_grow_data(struct elapi_data_out *out_data,
+ uint32_t len,
+ uint32_t block);
#endif
diff --git a/common/elapi/elapi_test/configure.ac b/common/elapi/elapi_test/configure.ac
index b0bbc17..44524e7 100644
--- a/common/elapi/elapi_test/configure.ac
+++ b/common/elapi/elapi_test/configure.ac
@@ -26,6 +26,7 @@ AC_ARG_ENABLE([verbose],
[AS_HELP_STRING([--enable-verbose],[build with verbose output])],
[AC_DEFINE([ELAPI_VERBOSE],[],[add verbose output])],[])
+m4_include(../def_macros.m4)
AC_CONFIG_FILES([Makefile])
AC_OUTPUT
diff --git a/common/elapi/providers/file/file_fmt_csv.c b/common/elapi/providers/file/file_fmt_csv.c
index e7d3d04..a811113 100644
--- a/common/elapi/providers/file/file_fmt_csv.c
+++ b/common/elapi/providers/file/file_fmt_csv.c
@@ -36,9 +36,9 @@
/* Calculate the potential size of the item */
static unsigned file_csv_data_len(struct file_csv_cfg *cfg,
int type,
- int length)
+ int raw_len)
{
- int len = 0;
+ int serialized_len = 0;
TRACE_FLOW_STRING("col_get_data_len", "Entry point");
@@ -47,40 +47,36 @@ static unsigned file_csv_data_len(struct file_csv_cfg *cfg,
case COL_TYPE_UNSIGNED:
case COL_TYPE_LONG:
case COL_TYPE_ULONG:
- if (cfg->csvqualifier) len = 20;
- else len = 22;
+ serialized_len = MAX_LONG_STRING_LEN;
break;
case COL_TYPE_STRING:
- if (cfg->csvqualifier) {
- if (cfg->csvescchar) len = length * 2 + 2;
- else len = length + 2;
- }
- else len = length;
+ if ((cfg->csvqualifier) &&
+ (cfg->csvescchar)) serialized_len = raw_len * 2;
+ else serialized_len = raw_len;
break;
+
case COL_TYPE_BINARY:
- len = length * 2;
+ serialized_len = raw_len * 2;
break;
case COL_TYPE_DOUBLE:
- /* If seems that 22 should be enough */
- if (cfg->csvqualifier) len = 22;
- else len = 24;
+ serialized_len = MAX_DOUBLE_STRING_LEN;
break;
case COL_TYPE_BOOL:
- if (cfg->csvqualifier) len = 5;
- else len = 3;
+ serialized_len = MAX_BOOL_STRING_LEN;
break;
default:
- len = 0;
+ serialized_len = 0;
break;
}
- TRACE_FLOW_STRING("col_get_data_len","Exit point");
+ if (cfg->csvqualifier) serialized_len += 2;
- return (uint32_t)len;
+ TRACE_FLOW_STRING("col_get_data_len","Exit point");
+ return (uint32_t)serialized_len;
}
/* Copy data escaping characters */
@@ -109,7 +105,7 @@ int file_copy_esc(char *dest,
}
/* Serialize item into the csv format */
-int file_serialize_csv(SEDO *out_data,
+int file_serialize_csv(struct elapi_data_out *out_data,
int type,
int length,
void *data,
@@ -117,7 +113,8 @@ int file_serialize_csv(SEDO *out_data,
{
int error = EOK;
struct file_csv_cfg *cfg;
- uint32_t len;
+ uint32_t projected_len;
+ uint32_t used_len;
int first = 1;
int i;
@@ -126,21 +123,25 @@ int file_serialize_csv(SEDO *out_data,
cfg = (struct file_csv_cfg *)mode_cfg;
/* Get projected length of the item */
- len = file_csv_data_len(cfg, type, length);
+ projected_len = file_csv_data_len(cfg, type, length);
- TRACE_INFO_NUMBER("Expected data length: ", len);
+ TRACE_INFO_NUMBER("Expected data length: ", projected_len);
/* Make sure we have enough space */
if (out_data->buffer != NULL) {
TRACE_INFO_STRING("Not a first time use.", "Adding length overhead");
- if (cfg->csvseparator) len++;
- len += cfg->csvnumsp;
+ if (cfg->csvseparator) projected_len++;
+ projected_len += cfg->csvnumsp;
first = 0;
}
+ else {
+ /* Add null terminating zero */
+ projected_len++;
+ }
/* Grow buffer if needed */
error = elapi_grow_data(out_data,
- len,
+ projected_len,
FILE_CSV_BLOCK);
if (error) {
TRACE_ERROR_NUMBER("Error. Failed to allocate memory.", error);
@@ -173,67 +174,68 @@ int file_serialize_csv(SEDO *out_data,
if ((cfg->csvqualifier) && (cfg->csvescchar)) {
/* Qualify and escape */
- len = file_copy_esc(&out_data->buffer[out_data->length],
- (const char *)(data),
- cfg->csvqualifier,
- cfg->csvescchar);
+ used_len = file_copy_esc((char *)&out_data->buffer[out_data->length],
+ (const char *)(data),
+ cfg->csvqualifier,
+ cfg->csvescchar);
}
else {
/* No escaping so just copy without trailing 0 */
/* Item's length includes trailing 0 for data items */
- len = length - 1;
+ used_len = length - 1;
memcpy(&out_data->buffer[out_data->length],
(const char *)(data),
- len);
+ used_len);
}
break;
case COL_TYPE_BINARY:
for (i = 0; i < length; i++)
- sprintf(&out_data->buffer[out_data->length + i * 2],
+ sprintf((char *)&out_data->buffer[out_data->length + i * 2],
"%02X", (unsigned int)(((const unsigned char *)(data))[i]));
- len = length * 2;
+ used_len = length * 2;
break;
case COL_TYPE_INTEGER:
- len = sprintf(&out_data->buffer[out_data->length],
- "%d", *((const int *)(data)));
+ used_len = sprintf((char *)&out_data->buffer[out_data->length],
+ "%d", *((const int *)(data)));
break;
case COL_TYPE_UNSIGNED:
- len = sprintf(&out_data->buffer[out_data->length],
- "%u", *((const unsigned int *)(data)));
+ used_len = sprintf((char *)&out_data->buffer[out_data->length],
+ "%u", *((const unsigned int *)(data)));
break;
case COL_TYPE_LONG:
- len = sprintf(&out_data->buffer[out_data->length],
- "%ld", *((const long *)(data)));
+ used_len = sprintf((char *)&out_data->buffer[out_data->length],
+ "%ld", *((const long *)(data)));
break;
case COL_TYPE_ULONG:
- len = sprintf(&out_data->buffer[out_data->length],
- "%lu", *((const unsigned long *)(data)));
+ used_len = sprintf((char *)&out_data->buffer[out_data->length],
+ "%lu", *((const unsigned long *)(data)));
break;
case COL_TYPE_DOUBLE:
- len = sprintf(&out_data->buffer[out_data->length],
- "%.4f", *((const double *)(data)));
+ used_len = sprintf((char *)&out_data->buffer[out_data->length],
+ "%.4f", *((const double *)(data)));
break;
case COL_TYPE_BOOL:
- len = sprintf(&out_data->buffer[out_data->length],
- "%s", (*((const unsigned char *)(data))) ? "yes" : "no");
+ used_len = sprintf((char *)&out_data->buffer[out_data->length],
+ "%s",
+ (*((const unsigned char *)(data))) ? "true" : "false");
break;
default:
out_data->buffer[out_data->length] = '\0';
- len = 0;
+ used_len = 0;
break;
}
/* Adjust length */
- out_data->length += len;
+ out_data->length += used_len;
/* Add qualifier */
if (cfg->csvqualifier) {
@@ -241,6 +243,10 @@ int file_serialize_csv(SEDO *out_data,
out_data->length++;
}
+ /* The "length" member of the structure does not account
+ * for the 0 symbol but we made sure that it fits
+ * when we asked for the memory at the top.
+ */
out_data->buffer[out_data->length] = '\0';
TRACE_INFO_STRING("Data: ", out_data->buffer);
@@ -291,7 +297,7 @@ int file_get_csv_cfg(void **storage,
/* Do we have qualifier? */
if (cfg_item == NULL) {
/* There is no qualifier - use default */
- cfg->csvqualifier = '"';
+ cfg->csvqualifier = FILE_CSV_DEF_QUAL;
}
else {
/* Get qualifier from configuration */
@@ -328,7 +334,7 @@ int file_get_csv_cfg(void **storage,
/* Do we have separator? */
if (cfg_item == NULL) {
/* There is no separator - use default */
- cfg->csvseparator = ',';
+ cfg->csvseparator = FILE_CSV_DEF_SEP;
}
else {
/* Get separator from configuration */
@@ -365,7 +371,7 @@ int file_get_csv_cfg(void **storage,
/* Do we have esc symbol? */
if (cfg_item == NULL) {
/* There is no esc symbol - use default */
- cfg->csvescchar = '\\';
+ cfg->csvescchar = FILE_CSV_DEF_ESC;
}
else {
/* Get esc symbol from configuration */
@@ -402,7 +408,7 @@ int file_get_csv_cfg(void **storage,
/* Do we have space? */
if (cfg_item == NULL) {
/* There is no esc symbol - use default */
- cfg->csvspace = ' ';
+ cfg->csvspace = FILE_CSV_DEF_SPC;
}
else {
/* Get file name from configuration */
diff --git a/common/elapi/providers/file/file_fmt_csv.h b/common/elapi/providers/file/file_fmt_csv.h
index 3e0e56d..6cc5274 100644
--- a/common/elapi/providers/file/file_fmt_csv.h
+++ b/common/elapi/providers/file/file_fmt_csv.h
@@ -38,6 +38,13 @@
#define FILE_CSV_TAB "tab"
#define FILE_CSV_CR "cr"
+
+/* Default values for configuration parameters */
+#define FILE_CSV_DEF_QUAL '"'
+#define FILE_CSV_DEF_SEP ','
+#define FILE_CSV_DEF_ESC '\\'
+#define FILE_CSV_DEF_SPC ' '
+
/* Try catch corrupted configuration 80 is more than enough */
#define FILE_MAXSPACE 80
@@ -60,7 +67,7 @@ int file_get_csv_cfg(void **storage,
const char *appname);
/* Serialize an item into the csv format */
-int file_serialize_csv(SEDO *out_data,
+int file_serialize_csv(struct elapi_data_out *out_data,
int type,
int length,
void *data,
diff --git a/common/elapi/providers/file/file_provider.c b/common/elapi/providers/file/file_provider.c
index 27747bc..09d6261 100644
--- a/common/elapi/providers/file/file_provider.c
+++ b/common/elapi/providers/file/file_provider.c
@@ -222,7 +222,7 @@ static int file_build_set(struct file_prvdr_cfg *file_cfg,
/* We found an end list field in the middle - error */
TRACE_ERROR_NUMBER("More fields after end list field.", EINVAL);
col_destroy_collection(set);
- free_string_config_array(fields);
+ free_string_config_array(fields);
return EINVAL;
}
@@ -244,7 +244,7 @@ static int file_build_set(struct file_prvdr_cfg *file_cfg,
/* Wrong format */
TRACE_ERROR_NUMBER("Leftover field has invalid format.", EINVAL);
col_destroy_collection(set);
- free_string_config_array(fields);
+ free_string_config_array(fields);
return EINVAL;
}
@@ -258,7 +258,7 @@ static int file_build_set(struct file_prvdr_cfg *file_cfg,
if (error) {
TRACE_ERROR_NUMBER("Error adding item to the set.", error);
col_destroy_collection(set);
- free_string_config_array(fields);
+ free_string_config_array(fields);
return error;
}
}
@@ -311,7 +311,7 @@ static int file_read_cfg(struct file_prvdr_cfg *file_cfg,
return error;
}
/* Check if file name is empty */
- if (filename[0] == '\0') use_default_name = 1;
+ if (filename[0] == '\0') use_default_name = 1;
else {
/* Now get a copy */
file_cfg->filename = get_string_config_value(cfg_item, &error);
@@ -596,11 +596,11 @@ int file_init(void **priv_ctx,
return error;
}
- /* Open file */
+ /* Open file */
/* FIXME: ... */
#ifdef ELAPI_VERBOSE
- printf("Initializaing file provider for sink: [%s]\n", name);
+ printf("Initializaing file provider for sink: [%s]\n", name);
file_print_ctx(*((struct file_prvdr_ctx **)priv_ctx));
#endif
@@ -631,7 +631,7 @@ int file_submit(void *priv_ctx, struct collection_item *event)
{
int error = EOK;
struct file_prvdr_ctx *ctx = (struct file_prvdr_ctx *)priv_ctx;
- SEDO *out_data;
+ struct elapi_data_out *out_data;
TRACE_FLOW_STRING("file_submit", "Entry point");
diff --git a/common/elapi/providers/file/file_util.c b/common/elapi/providers/file/file_util.c
index 22214f3..4508e35 100644
--- a/common/elapi/providers/file/file_util.c
+++ b/common/elapi/providers/file/file_util.c
@@ -210,7 +210,7 @@ static int file_split_by_set(struct collection_item **leftovers,
}
/* Function to serialize one item */
-static int file_serialize_item(SEDO *out_data,
+static int file_serialize_item(struct elapi_data_out *out_data,
int type,
int length,
void *data,
@@ -273,7 +273,7 @@ static int file_serialize_item(SEDO *out_data,
/* Function to serialize the list */
-static int file_serialize_list(SEDO **out_data,
+static int file_serialize_list(struct elapi_data_out **out_data,
int append,
int reference,
struct collection_item *input,
@@ -281,8 +281,8 @@ static int file_serialize_list(SEDO **out_data,
void *mode_cfg)
{
int error = EOK;
- SEDO *allocated = NULL;
- SEDO *to_use = NULL;
+ struct elapi_data_out *allocated = NULL;
+ struct elapi_data_out *to_use = NULL;
struct collection_iterator *iterator;
struct collection_item *item;
@@ -381,13 +381,13 @@ static int file_serialize_list(SEDO **out_data,
}
/* Function to log event into sink */
-int file_prep_data(SEDO **out_data,
+int file_prep_data(struct elapi_data_out **out_data,
struct file_prvdr_ctx *ctx,
struct collection_item *event)
{
int error = EOK;
- SEDO *serialized = NULL;
- SEDO *serialized_lo = NULL;
+ struct elapi_data_out *serialized = NULL;
+ struct elapi_data_out *serialized_lo = NULL;
struct collection_item *leftovers = NULL;
TRACE_FLOW_STRING("file_prep_data", "Entry");
diff --git a/common/elapi/providers/file/file_util.h b/common/elapi/providers/file/file_util.h
index 4e4eb7c..f4c0e4b 100644
--- a/common/elapi/providers/file/file_util.h
+++ b/common/elapi/providers/file/file_util.h
@@ -41,7 +41,7 @@
/* Function to prepare data for logging */
-int file_prep_data(SEDO **out_data,
+int file_prep_data(struct elapi_data_out **out_data,
struct file_prvdr_ctx *ctx,
struct collection_item *event);
--
1.5.5.6
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
