Bugfix in logger + test for logger format parsing

include/logger.h

@@ -52,6 +52,8 @@
 /**@ingroup log_facility */
 #define LOG_MASTER 8
 
+#define PYFCGI_LOGGER_MAX 64
+#define PYFCGI_LOGGER_FMT_MAX PYFCGI_LOGGER_MAX
 #define PYFCGI_LOGGER_FIELD_MAX 24
 #define PYFCGI_LOGGER_FMT_PARSE_ERRSZ 64
 
@@ -151,45 +153,7 @@ static const char* PYFCGI_LOGGER_LVLNAME[] = {"Emergency", "Alert", "Critical",
 
 static const char* PYFCGI_LOGGER_TYPNAME[] = {"Global", "Access", "Internal",
                                               "Worker", "Master"};
-/**@brief Logger configuration
- * @ingroup conf_logger
- */
-struct pyfcgi_conf_logger_s
-{
-	/**@brief Or combination of @ref PYFCGI_LOG_SYSLOG or
-	 * @ref PYFCGI_LOG_RETRY */
-	short flags;
-	char *syslog_ident;
-	int syslog_facility;
-	logmask_t syslog_loglvl;
-	logmask_t syslog_logtyp;
 
-	/**@brief PyFCGI internal ident, prefixes all log messages */
-	char *ident;
-	logmask_t facility;
-
-	pyfcgi_logger_t *loggers;
-	unsigned char logger_sz;
-
-	char **format;
-	unsigned char format_sz;
-
-	/**@brief Internal pipe to tee(2) the message on loggers */
-	int pipes[2];
-};
-
-/**@brief Informations on a logger
- * @ingroup conf_logger
- */
-struct pyfcgi_logger_s
-{
-	char *filename;
-	int fd;
-	logmask_t loglvl;
-	logmask_t logtyp;
-
-	size_t fmt_id;
-};
 
 /**@defgroup conf_logger_format Logline format
  * @ingroup conf_logger
@@ -263,8 +227,12 @@ struct pyfcgi_logger_fmt_field_s
  * @ingroup conf_logger_format */
 struct pyfcgi_logger_format_s
 {
+	/**@brief String copy of the format */
+	char *fmt;
 	/**@brief Stores data about fields */
 	pyfcgi_logger_fmt_field_t fields[PYFCGI_LOGGER_FIELD_MAX];
+	/**@brief field count */
+	int nfield;
 	/**@brief Preallocated buffer for prefix & suffix */
 	char *buf;
 	/**@brief Message prefix */
@@ -273,8 +241,49 @@ struct pyfcgi_logger_format_s
 	char *suffix;
 };
 
+/**@brief Informations on a logger
+ * @ingroup conf_logger
+ */
+struct pyfcgi_logger_s
+{
+	char *filename;
+	int fd;
+	logmask_t loglvl;
+	logmask_t logtyp;
+
+	size_t fmt_id;
+};
+
+/**@brief Logger configuration
+ * @ingroup conf_logger
+ */
+struct pyfcgi_conf_logger_s
+{
+	/**@brief Or combination of @ref PYFCGI_LOG_SYSLOG or
+	 * @ref PYFCGI_LOG_RETRY */
+	short flags;
+	char *syslog_ident;
+	int syslog_facility;
+	logmask_t syslog_loglvl;
+	logmask_t syslog_logtyp;
+
+	/**@brief PyFCGI internal ident, prefixes all log messages */
+	char *ident;
+	logmask_t facility;
+
+	pyfcgi_logger_t loggers[PYFCGI_LOGGER_MAX];
+	unsigned char logger_sz;
+
+	pyfcgi_logger_format_t formats[PYFCGI_LOGGER_MAX];
+	//char **format;
+	unsigned char format_sz;
+
+	/**@brief Internal pipe to tee(2) the message on loggers */
+	int pipes[2];
+};
 #include "conf.h"
 
+/**@brief Initiliaze logger conf */
 int pyfcgi_logger_init();
 
 /**@brief Add a new logger

src/logger.c

@@ -20,7 +20,7 @@ int pyfcgi_logger_add(const char *filename, logmask_t loglvl, logmask_t logtyp,
 
 	conf = &PyFCGI_conf.logs;
 
-	if(conf->logger_sz == 255)
+	if(conf->logger_sz >= PYFCGI_LOGGER_MAX)
 	{
 		pyfcgi_log(LOG_ERR, "Maximum of 255 logger reached, unable to add this one...");
 		return PYFCGI_ERR;
@@ -64,41 +64,22 @@ err:
 
 int pyfcgi_logger_format_add(const char* format, size_t* idx)
 {
-	char *fmt;
 	size_t i;
-	//void *tmp;
 	pyfcgi_conf_logger_t *conf;
 
 	conf = &PyFCGI_conf.logs;
 
 	for(i=0; i<conf->format_sz; i++)
 	{
-		if(!strcmp(format, conf->format[i])) { break; }
+		if(!strcmp(format, conf->formats[i].fmt)) { break; }
 	}
 	if(idx) { *idx = i; }
 	if( i<conf->format_sz )
 	{
 		return 0;
 	}
-	if( !(fmt = strdup(format)) )
-	{
-		pyfcgi_log(LOG_ALERT, "Fails to strdup new format : %s",
-			   strerror(errno));
-		return PYFCGI_FATAL;
-	}
-	/*
 	conf->format_sz++;
-	if( !(tmp = realloc(conf->format,
-	                    sizeof(char*) * conf->format_sz)) )
-	{
-		conf->format_sz--;
-		pyfcgi_log(LOG_ALERT, "Fails to realloc logger format array : %s",
-			   strerror(errno));
-		return PYFCGI_FATAL;
-	}
-	conf->format[i] = fmt;
-	*/
-	return 0;
+	return pyfcgi_logger_parse_format(format, &(conf->formats[i]));
 }
 
 int pyfcgi_logger_parse_format(const char* fmt,
@@ -107,31 +88,34 @@ int pyfcgi_logger_parse_format(const char* fmt,
 	const char *ptr, *str_start;
 	int err;
 	char reason_err[PYFCGI_LOGGER_FMT_PARSE_ERRSZ];
-	unsigned char nfield;
 	pyfcgi_logger_fmt_field_t *cur_field;
 
 	memset(fmt_data, 0, sizeof(pyfcgi_logger_format_t));
+
+	if( !(fmt_data->fmt = strdup(fmt)) )
+	{
+		pyfcgi_log(LOG_ALERT, "Fails to strdup new format : %s",
+			   strerror(errno));
+		return PYFCGI_FATAL;
+	}
+
 	ptr = fmt;
-	nfield = 0;
+	fmt_data->nfield = 0;
 	cur_field = fmt_data->fields;
 
 	while(*ptr)
 	{
-		if(nfield >= PYFCGI_LOGGER_FIELD_MAX)
+		if(fmt_data->nfield >= PYFCGI_LOGGER_FIELD_MAX)
 		{
 			//TODO error truncate !
-			break;
+			//break; 
+			goto parse_err;
 		}
 		str_start = ptr;
-		while(*ptr && *ptr != '{' && *(ptr+1) != '{')
+		cur_field->len = 0;
+		while(*ptr && (*ptr != '{' || *(ptr+1) == '{'))
 		{ // handling const chr fields
 			//TODO : issue warning/error when unescaped '}' found
-			if(!cur_field->type)
-			{
-				cur_field->type = pyfcgi_logger_field_const;
-				cur_field->known_length = 1;
-				cur_field->len = 0;
-			}
 			if(*ptr == '{' && *(ptr+1) == '{')
 			{
 				cur_field->len++;
@@ -142,6 +126,8 @@ int pyfcgi_logger_parse_format(const char* fmt,
 		}
 		if(cur_field->len)
 		{
+			cur_field->known_length = 1;
+			cur_field->type = pyfcgi_logger_field_const;
 			cur_field->val = strndup(str_start, cur_field->len);
 			if(!cur_field->val)
 			{
@@ -151,19 +137,20 @@ int pyfcgi_logger_parse_format(const char* fmt,
 					   strerror(err));
 				goto exit_err;
 			}
-			nfield++;
-			cur_field = &(fmt_data->fields[nfield]); // next_field
-			continue; // to check nfield
+			fmt_data->nfield++;
+			cur_field = &(fmt_data->fields[fmt_data->nfield]); // next_field
+			continue; // to check fmt_data->nfield
 		}
 		if(*ptr == '{') //should be this or '\0'...
 		{
 			ptr++;
-			if( (1 == pyfcgi_logger_parse_field(&ptr, fmt, 
-			                                    cur_field, 
-			                                    reason_err)) )
+			if( pyfcgi_logger_parse_field(&ptr, fmt,  cur_field, 
+			                              reason_err) )
 			{
 				goto parse_err;
 			}
+			fmt_data->nfield++;
+			cur_field = &(fmt_data->fields[fmt_data->nfield]); // next_field
 		}
 		ptr++;
 	};
@@ -336,7 +323,6 @@ int pyfcgi_logger_parse_field_dtfmt(const char** ptr, char** format)
 	}
 	if(!fmt_len)
 	{
-		(*ptr)++;
 		fmt_len = sizeof(PYFCGI_LOGGER_TIME_FMT_DEFAULT);
 		fmt = PYFCGI_LOGGER_TIME_FMT_DEFAULT;
 	}
@@ -356,7 +342,6 @@ int pyfcgi_logger_parse_field_sz(const char **ptr, size_t *size)
 	*size = 0;
 	if(**ptr == '}' || **ptr == ':')
 	{
-		(*ptr)++;
 		return 0;
 	}
 

tests/check_logger.c

@@ -15,8 +15,10 @@ START_TEST(test_logger_init)
 	pyfcgi_logger_init();
 	ck_assert_ptr_eq(conf->syslog_ident, NULL);
 	ck_assert_ptr_eq(conf->ident, NULL);
-	ck_assert_ptr_eq(conf->format, NULL);
+	/*
+	ck_assert_ptr_eq(conf->formats, NULL);
 	ck_assert_ptr_eq(conf->loggers, NULL);
+	*/
 	ck_assert_int_eq(conf->logger_sz, 0);
 	ck_assert_int_eq(conf->format_sz, 0);
 	ck_assert_int_eq(conf->flags, 0);
@@ -24,19 +26,20 @@ START_TEST(test_logger_init)
 }
 END_TEST
 
-/*
 START_TEST(test_logger_format_add)
 {
 	int ret;
 	size_t s;
 	pyfcgi_logger_init();
+	const char *fmt = "{datetime} {level} {ident}[{pid}] {msg}";
 
-	ret = pyfcgi_logger_format_add("TEST", NULL);
+	ret = pyfcgi_logger_format_add(fmt, &s);
 	ck_assert_int_eq(conf->format_sz, 1);
-	ck_assert_str_eq(conf->format[0], "TEST");
+	ck_assert_int_eq(s, 0);
+	ck_assert_str_eq(conf->formats[s].fmt, fmt);
+
 }
 END_TEST
-*/
 
 /*
 	TC logger default values
@@ -54,7 +57,7 @@ START_TEST(test_logger_parse_field_dtfmt_default)
 	ret = pyfcgi_logger_parse_field_dtfmt((const char**)&ptr, &fmt);
 	ck_assert_int_eq(ret, 0);
 	ck_assert_str_eq(fmt, PYFCGI_LOGGER_TIME_FMT_DEFAULT);
-	ck_assert_ptr_eq(ptr, orig+1);
+	ck_assert_ptr_eq(ptr, orig);
 	free(fmt);
 }
 END_TEST
@@ -74,7 +77,7 @@ START_TEST(test_logger_parse_field_sz_default)
 		ret = pyfcgi_logger_parse_field_sz((const char**)&ptr, &val);
 		ck_assert_int_eq(ret, 0);
 		ck_assert_int_eq(val, 0);
-		ck_assert_ptr_eq(ptr, (*orig)+1);
+		ck_assert_ptr_eq(ptr, (*orig));
 		orig++;
 	}
 }
@@ -122,11 +125,13 @@ START_TEST(test_logger_parse_field_sz)
 {
 	int ret, *res;
 	size_t val;
-	char *ptr, **orig;
-	char *vals[] = {"42:%F}", "1337:}", "0}", NULL};
-	int _res[] = {42,1337,0};
+	char *ptr, **orig, **eptr;
+	char *vals[] = {"42:%F}", "1337:}", "0}", ":}", NULL};
+	int _res[] = {42,1337,0, 0};
+	char *_eptr[] = {vals[0]+2, vals[1]+4, vals[2]+1, vals[3]};
 
 	orig = vals;
+	eptr = _eptr;
 	res = _res;
 	while(*orig)
 	{
@@ -134,9 +139,10 @@ START_TEST(test_logger_parse_field_sz)
 
 		ret = pyfcgi_logger_parse_field_sz((const char**)&ptr, &val);
 		ck_assert_int_eq(val, *res);
-		ck_assert_ptr_eq(ptr, (*orig)+snprintf(NULL, 0, "%d", *res));
+		ck_assert_ptr_eq(ptr, *eptr);
 		orig++;
 		res++;
+		eptr++;
 	}
 }
 END_TEST
@@ -174,10 +180,16 @@ START_TEST(test_logger_parse_field_datetime)
 	pyfcgi_logger_fmt_field_t field;
 	char *vals[] = {"datetime:42}", "datetime::%s%F}",
 	                "datetime:42:COUCOU}",
-			"dat:32:FOO}", "d::BAR}", NULL};
-	int _len[] = {42, PYFCGI_LOG_DTM_LEN, 42, 32, PYFCGI_LOG_DTM_LEN};
+			"dat:32:FOO}", "d::BAR}",
+			"datetime}{level}", "datetime} {level}",
+			"datetime}:{level}",
+			NULL};
+	int _len[] = {42, PYFCGI_LOG_DTM_LEN, 42, 32, PYFCGI_LOG_DTM_LEN,
+	              PYFCGI_LOG_DTM_LEN, PYFCGI_LOG_DTM_LEN, PYFCGI_LOG_DTM_LEN};
 	char *_fmt[] = {PYFCGI_LOGGER_TIME_FMT_DEFAULT, "%s%F", "COUCOU", "FOO",
-	                "BAR"};
+	                "BAR", PYFCGI_LOGGER_TIME_FMT_DEFAULT,
+			PYFCGI_LOGGER_TIME_FMT_DEFAULT,
+			PYFCGI_LOGGER_TIME_FMT_DEFAULT};
 
 	memset(&field, 0, sizeof(pyfcgi_logger_fmt_field_t));
 	
@@ -339,8 +351,9 @@ START_TEST(test_logger_parse_field_msg)
 	int ret;
 	char **val, **ptr;
 	char err[PYFCGI_LOGGER_FMT_PARSE_ERRSZ];
-	char *vals[] = {"msg}", "m}", "ms}", NULL};
 	pyfcgi_logger_fmt_field_t field;
+	char *vals[] = {"msg}", "m}", "ms}", NULL};
+
 
 	val = vals;
 	memset(&field, 0, sizeof(pyfcgi_logger_fmt_field_t));
@@ -365,6 +378,35 @@ START_TEST(test_logger_parse_field_msg)
 }
 END_TEST
 
+START_TEST(test_logger_format_parse1)
+{
+	int ret, i;
+	size_t s;
+	pyfcgi_logger_init();
+	const char *fmt = "{datetime} {level} {ident}[{pid}] {msg}";
+	pyfcgi_logger_field_type_e types[] = {
+		pyfcgi_logger_field_datetime, pyfcgi_logger_field_const,
+		pyfcgi_logger_field_level, pyfcgi_logger_field_const,
+		pyfcgi_logger_field_ident, pyfcgi_logger_field_const,
+		pyfcgi_logger_field_pid, pyfcgi_logger_field_const,
+		pyfcgi_logger_field_msg};
+
+
+	ret = pyfcgi_logger_format_add(fmt, &s);
+	ck_assert_int_eq(ret, 0);
+	ck_assert_int_eq(conf->format_sz, 1);
+	ck_assert_int_eq(s, 0);
+	ck_assert_str_eq(conf->formats[s].fmt, fmt);
+	for(i=0; i<9; i++)
+	{
+dprintf(2, "I=%d\n", i);
+		ck_assert_int_eq(conf->formats[s].fields[i].type, types[i]);
+	}
+	ck_assert_int_eq(conf->formats[s].nfield, 9);
+
+}
+END_TEST
+
 /*
 	TC logger format parsing error
 */
@@ -425,11 +467,6 @@ Suite * logger_suite(void)
 
 	s = suite_create("Logger");
 
-	tc_init = tcase_create("Initialisation");
-	tcase_add_test(tc_init, test_logger_init);
-	//tcase_add_test(tc_init, test_logger_format_add);
-	suite_add_tcase(s, tc_init);
-
 	tc_field_default = tcase_create("Logger format parsing default values");
 	tcase_add_test(tc_field_default, test_logger_parse_field_dtfmt_default);
 	tcase_add_test(tc_field_default, test_logger_parse_field_sz_default);
@@ -444,6 +481,7 @@ Suite * logger_suite(void)
 	tcase_add_test(tc_field, test_logger_parse_field_pid);
 	tcase_add_test(tc_field, test_logger_parse_field_ident);
 	tcase_add_test(tc_field, test_logger_parse_field_msg);
+	tcase_add_test(tc_field, test_logger_format_parse1);
 	suite_add_tcase(s, tc_field);
 
 	tc_parse_err = tcase_create("Logger format parsing errors handling");
@@ -451,6 +489,11 @@ Suite * logger_suite(void)
 	tcase_add_test(tc_parse_err, test_logger_parse_field_parse_err);
 	suite_add_tcase(s, tc_parse_err);
 	
+	tc_init = tcase_create("Initialisation");
+	tcase_add_test(tc_init, test_logger_init);
+	tcase_add_test(tc_init, test_logger_format_add);
+	suite_add_tcase(s, tc_init);
+
 
 	return s;
 }