diff mbox

[1/6] lib: fwts_summary: remove references to log line number

Message ID 1340191829-27444-2-git-send-email-colin.king@canonical.com
State Accepted
Headers show

Commit Message

Colin Ian King June 20, 2012, 11:30 a.m. UTC 
From: Colin Ian King <colin.king@canonical.com>

With the changes to the logging back end, line numbers vary depending
on the output back-end being used.  So, referencing the log line number
is no longer trivial.  Removing the log line references in the summary
is probably the best way forward for the moment, otherwise we end up
writing a really overly complex log line tracking mechanism for something
that isn't too useful anyhow.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/lib/src/fwts_summary.c |   57 ++++++--------------------------------------
 1 file changed, 7 insertions(+), 50 deletions(-)

Comments

Keng-Yu Lin June 20, 2012, 3 p.m. UTC | #1 
On Wed, Jun 20, 2012 at 7:30 PM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> With the changes to the logging back end, line numbers vary depending
> on the output back-end being used.  So, referencing the log line number
> is no longer trivial.  Removing the log line references in the summary
> is probably the best way forward for the moment, otherwise we end up
> writing a really overly complex log line tracking mechanism for something
> that isn't too useful anyhow.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/lib/src/fwts_summary.c |   57 ++++++--------------------------------------
>  1 file changed, 7 insertions(+), 50 deletions(-)
>
> diff --git a/src/lib/src/fwts_summary.c b/src/lib/src/fwts_summary.c
> index b8fe644..5c401b1 100644
> --- a/src/lib/src/fwts_summary.c
> +++ b/src/lib/src/fwts_summary.c
> @@ -26,7 +26,6 @@
>  typedef struct {
>        char *test;             /* test that found the error */
>        char *text;             /* text of failure message */
> -       fwts_list log_lines;    /* line where failure was reported */
>  } fwts_summary_item;
>
>  enum {
> @@ -78,7 +77,6 @@ static void fwts_summary_item_free(void *data)
>  {
>        fwts_summary_item *item = (fwts_summary_item *)data;
>
> -       fwts_list_free_items(&(item->log_lines), free);
>        free(item->test);
>        free(item->text);
>        free(item);
> @@ -124,10 +122,6 @@ int fwts_summary_add(fwts_framework *fw, const char *test, fwts_log_level level,
>        fwts_summary_item *summary_item = NULL;
>        bool summary_item_found = false;
>        int index = fwts_summary_level_to_index(level);
> -       int *line_num;
> -
> -       if ((line_num = calloc(1, sizeof(int))) == NULL)
> -               return FWTS_ERROR;
>
>        /* Does the text already exist? - search for it */
>        fwts_list_foreach(item, fwts_summaries[index]) {
> @@ -140,30 +134,22 @@ int fwts_summary_add(fwts_framework *fw, const char *test, fwts_log_level level,
>
>        /* Not found, create a new one */
>        if (!summary_item_found) {
> -               if ((summary_item = calloc(1, sizeof(fwts_summary_item))) == NULL) {
> -                       free(line_num);
> +               if ((summary_item = calloc(1, sizeof(fwts_summary_item))) == NULL)
>                        return FWTS_ERROR;
> -               }
> +
>                if ((summary_item->test = strdup(test)) == NULL) {
> -                       free(line_num);
>                        free(summary_item);
>                        return FWTS_ERROR;
>                }
> +
>                if ((summary_item->text = strdup(text)) == NULL) {
> -                       free(line_num);
> -                       free(summary_item->test);
> -                       free(summary_item);
> +                       free(summary_item->test);
> +                       free(summary_item);
>                        return FWTS_ERROR;
>                }
> -               fwts_list_init(&(summary_item->log_lines));
>                fwts_chop_newline(summary_item->text);
>        }
>
> -       /* Now append a new line number to list of line numbers */
> -
> -       *line_num = fwts_log_line_number(fw->results);
> -       fwts_list_append(&summary_item->log_lines, line_num);
> -
>        /* And append new item if not done so already */
>        if (!summary_item_found)
>                fwts_list_append(fwts_summaries[index], summary_item);
> @@ -179,26 +165,6 @@ static void fwts_summary_format_field(char *buffer, int buflen, uint32_t value)
>                *buffer = '\0';
>  }
>
> -static char *fwts_summary_lines(fwts_list *list)
> -{
> -       fwts_list_link *item;
> -       char *text = NULL;
> -       char tmp[16];
> -       int i = 0;
> -
> -       fwts_list_foreach(item, list) {
> -               int *num = fwts_list_data(int *, item);
> -               snprintf(tmp, sizeof(tmp), "%s%d",
> -                       text == NULL ? "" : ", ", *num);
> -               text = fwts_realloc_strcat(text, tmp);
> -               if (i++ > 20) {
> -                       text = fwts_realloc_strcat(text, "...");
> -                       break;
> -               }
> -       }
> -       return text;
> -}
> -
>  /*
>  *  fwts_summary_report()
>  *     report test failure summary, sorted by error levels
> @@ -223,28 +189,19 @@ int fwts_summary_report(fwts_framework *fw, fwts_list *test_list)
>                        fwts_log_section_begin(fw->results, "failures");
>                        fwts_list_foreach(item, fwts_summaries[i]) {
>                                fwts_summary_item *summary_item = fwts_list_data(fwts_summary_item *,item);
> -                               char *lines = fwts_summary_lines(&summary_item->log_lines);
>
>                                /*
>                                 *  This is not pleasant, we really don't want very wide lines
>                                 *  logged in the HTML format, where we don't mind for other formats.
>                                 */
>                                if (fw->log_type == LOG_TYPE_HTML)
> -                                       fwts_log_summary(fw, " %s test, at %d log line%s: %s: %s",
> +                                       fwts_log_summary(fw, " %s test: %s",
>                                                summary_item->test,
> -                                               fwts_list_len(&summary_item->log_lines),
> -                                               fwts_list_len(&summary_item->log_lines) > 1 ? "s" : "",
> -                                               lines,
>                                                summary_item->text);
>                                else
> -                                       fwts_log_summary_verbatum(fw, " %s test, at %d log line%s: %s: %s",
> +                                       fwts_log_summary_verbatum(fw, " %s: %s",
>                                                summary_item->test,
> -                                               fwts_list_len(&summary_item->log_lines),
> -                                               fwts_list_len(&summary_item->log_lines) > 1 ? "s" : "",
> -                                               lines,
>                                                summary_item->text);
> -
> -                               free(lines);
>                        }
>                        fwts_log_section_end(fw->results);
>                }
> --
> 1.7.10.4
>
Acked-by: Keng-Yu Lin <kengyu@canonical.com>
Alex Hung June 21, 2012, 1:03 a.m. UTC | #2 
On 06/20/2012 07:30 PM, Colin King wrote:
> From: Colin Ian King<colin.king@canonical.com>
>
> With the changes to the logging back end, line numbers vary depending
> on the output back-end being used.  So, referencing the log line number
> is no longer trivial.  Removing the log line references in the summary
> is probably the best way forward for the moment, otherwise we end up
> writing a really overly complex log line tracking mechanism for something
> that isn't too useful anyhow.
>
> Signed-off-by: Colin Ian King<colin.king@canonical.com>
> ---
>   src/lib/src/fwts_summary.c |   57 ++++++--------------------------------------
>   1 file changed, 7 insertions(+), 50 deletions(-)
>
> diff --git a/src/lib/src/fwts_summary.c b/src/lib/src/fwts_summary.c
> index b8fe644..5c401b1 100644
> --- a/src/lib/src/fwts_summary.c
> +++ b/src/lib/src/fwts_summary.c
> @@ -26,7 +26,6 @@
>   typedef struct {
>   	char *test;		/* test that found the error */
>   	char *text;		/* text of failure message */
> -	fwts_list log_lines;	/* line where failure was reported */
>   } fwts_summary_item;
>
>   enum {
> @@ -78,7 +77,6 @@ static void fwts_summary_item_free(void *data)
>   {
>   	fwts_summary_item *item = (fwts_summary_item *)data;
>
> -	fwts_list_free_items(&(item->log_lines), free);
>   	free(item->test);
>   	free(item->text);
>   	free(item);
> @@ -124,10 +122,6 @@ int fwts_summary_add(fwts_framework *fw, const char *test, fwts_log_level level,
>   	fwts_summary_item *summary_item = NULL;
>   	bool summary_item_found = false;
>   	int index = fwts_summary_level_to_index(level);
> -	int *line_num;
> -
> -	if ((line_num = calloc(1, sizeof(int))) == NULL)
> -		return FWTS_ERROR;
>
>   	/* Does the text already exist? - search for it */
>   	fwts_list_foreach(item, fwts_summaries[index]) {
> @@ -140,30 +134,22 @@ int fwts_summary_add(fwts_framework *fw, const char *test, fwts_log_level level,
>
>   	/* Not found, create a new one */
>   	if (!summary_item_found) {
> -		if ((summary_item = calloc(1, sizeof(fwts_summary_item))) == NULL) {
> -			free(line_num);
> +		if ((summary_item = calloc(1, sizeof(fwts_summary_item))) == NULL)
>   			return FWTS_ERROR;
> -		}
> +
>   		if ((summary_item->test = strdup(test)) == NULL) {
> -			free(line_num);
>   			free(summary_item);
>   			return FWTS_ERROR;
>   		}
> +
>   		if ((summary_item->text = strdup(text)) == NULL) {
> -			free(line_num);
> -			free(summary_item->test);	
> -			free(summary_item);	
> +			free(summary_item->test);
> +			free(summary_item);
>   			return FWTS_ERROR;
>   		}
> -		fwts_list_init(&(summary_item->log_lines));
>   		fwts_chop_newline(summary_item->text);
>   	}
>
> -	/* Now append a new line number to list of line numbers */
> -
> -	*line_num = fwts_log_line_number(fw->results);
> -	fwts_list_append(&summary_item->log_lines, line_num);
> -
>   	/* And append new item if not done so already */
>   	if (!summary_item_found)
>   		fwts_list_append(fwts_summaries[index], summary_item);
> @@ -179,26 +165,6 @@ static void fwts_summary_format_field(char *buffer, int buflen, uint32_t value)
>   		*buffer = '\0';
>   }
>
> -static char *fwts_summary_lines(fwts_list *list)
> -{
> -	fwts_list_link *item;
> -	char *text = NULL;
> -	char tmp[16];
> -	int i = 0;
> -
> -	fwts_list_foreach(item, list) {
> -		int *num = fwts_list_data(int *, item);
> -		snprintf(tmp, sizeof(tmp), "%s%d",
> -			text == NULL ? "" : ", ", *num);
> -		text = fwts_realloc_strcat(text, tmp);
> -		if (i++>  20) {
> -			text = fwts_realloc_strcat(text, "...");
> -			break;
> -		}
> -	}
> -	return text;
> -}
> -
>   /*
>    *  fwts_summary_report()
>    *  	report test failure summary, sorted by error levels
> @@ -223,28 +189,19 @@ int fwts_summary_report(fwts_framework *fw, fwts_list *test_list)
>   			fwts_log_section_begin(fw->results, "failures");
>   			fwts_list_foreach(item, fwts_summaries[i]) {
>   				fwts_summary_item *summary_item = fwts_list_data(fwts_summary_item *,item);
> -				char *lines = fwts_summary_lines(&summary_item->log_lines);
>
>   				/*
>   				 *  This is not pleasant, we really don't want very wide lines
>   				 *  logged in the HTML format, where we don't mind for other formats.
>   				 */
>   				if (fw->log_type == LOG_TYPE_HTML)
> -					fwts_log_summary(fw, " %s test, at %d log line%s: %s: %s",
> +					fwts_log_summary(fw, " %s test: %s",
>   						summary_item->test,
> -						fwts_list_len(&summary_item->log_lines),
> -						fwts_list_len(&summary_item->log_lines)>  1 ? "s" : "",
> -						lines,
>   						summary_item->text);
>   				else
> -					fwts_log_summary_verbatum(fw, " %s test, at %d log line%s: %s: %s",
> +					fwts_log_summary_verbatum(fw, " %s: %s",
>   						summary_item->test,
> -						fwts_list_len(&summary_item->log_lines),
> -						fwts_list_len(&summary_item->log_lines)>  1 ? "s" : "",
> -						lines,
>   						summary_item->text);
> -
> -				free(lines);
>   			}
>   			fwts_log_section_end(fw->results);
>   		}

Acked-by: Alex Hung <alex.hung@canonical.com>
diff mbox

Patch

diff --git a/src/lib/src/fwts_summary.c b/src/lib/src/fwts_summary.c
index b8fe644..5c401b1 100644
--- a/src/lib/src/fwts_summary.c
+++ b/src/lib/src/fwts_summary.c
@@ -26,7 +26,6 @@ 
 typedef struct {
 	char *test;		/* test that found the error */
 	char *text;		/* text of failure message */
-	fwts_list log_lines;	/* line where failure was reported */
 } fwts_summary_item;
 
 enum {
@@ -78,7 +77,6 @@  static void fwts_summary_item_free(void *data)
 {
 	fwts_summary_item *item = (fwts_summary_item *)data;
 
-	fwts_list_free_items(&(item->log_lines), free);
 	free(item->test);
 	free(item->text);
 	free(item);
@@ -124,10 +122,6 @@  int fwts_summary_add(fwts_framework *fw, const char *test, fwts_log_level level,
 	fwts_summary_item *summary_item = NULL;
 	bool summary_item_found = false;
 	int index = fwts_summary_level_to_index(level);
-	int *line_num;
-
-	if ((line_num = calloc(1, sizeof(int))) == NULL)
-		return FWTS_ERROR;
 
 	/* Does the text already exist? - search for it */
 	fwts_list_foreach(item, fwts_summaries[index]) {
@@ -140,30 +134,22 @@  int fwts_summary_add(fwts_framework *fw, const char *test, fwts_log_level level,
 
 	/* Not found, create a new one */
 	if (!summary_item_found) {
-		if ((summary_item = calloc(1, sizeof(fwts_summary_item))) == NULL) {
-			free(line_num);
+		if ((summary_item = calloc(1, sizeof(fwts_summary_item))) == NULL)
 			return FWTS_ERROR;
-		}
+
 		if ((summary_item->test = strdup(test)) == NULL) {
-			free(line_num);
 			free(summary_item);
 			return FWTS_ERROR;
 		}
+
 		if ((summary_item->text = strdup(text)) == NULL) {
-			free(line_num);
-			free(summary_item->test);	
-			free(summary_item);	
+			free(summary_item->test);
+			free(summary_item);
 			return FWTS_ERROR;
 		}
-		fwts_list_init(&(summary_item->log_lines));
 		fwts_chop_newline(summary_item->text);
 	}
 
-	/* Now append a new line number to list of line numbers */
-
-	*line_num = fwts_log_line_number(fw->results);
-	fwts_list_append(&summary_item->log_lines, line_num);
-
 	/* And append new item if not done so already */
 	if (!summary_item_found)
 		fwts_list_append(fwts_summaries[index], summary_item);
@@ -179,26 +165,6 @@  static void fwts_summary_format_field(char *buffer, int buflen, uint32_t value)
 		*buffer = '\0';
 }
 
-static char *fwts_summary_lines(fwts_list *list)
-{
-	fwts_list_link *item;
-	char *text = NULL;
-	char tmp[16];
-	int i = 0;
-
-	fwts_list_foreach(item, list) {
-		int *num = fwts_list_data(int *, item);
-		snprintf(tmp, sizeof(tmp), "%s%d", 
-			text == NULL ? "" : ", ", *num);
-		text = fwts_realloc_strcat(text, tmp);
-		if (i++ > 20) {
-			text = fwts_realloc_strcat(text, "...");
-			break;
-		}
-	}
-	return text;
-}
-
 /*
  *  fwts_summary_report()
  *  	report test failure summary, sorted by error levels
@@ -223,28 +189,19 @@  int fwts_summary_report(fwts_framework *fw, fwts_list *test_list)
 			fwts_log_section_begin(fw->results, "failures");
 			fwts_list_foreach(item, fwts_summaries[i]) {
 				fwts_summary_item *summary_item = fwts_list_data(fwts_summary_item *,item);
-				char *lines = fwts_summary_lines(&summary_item->log_lines);
 
 				/*
 				 *  This is not pleasant, we really don't want very wide lines
 				 *  logged in the HTML format, where we don't mind for other formats.
 				 */
 				if (fw->log_type == LOG_TYPE_HTML)
-					fwts_log_summary(fw, " %s test, at %d log line%s: %s: %s",
+					fwts_log_summary(fw, " %s test: %s",
 						summary_item->test,
-						fwts_list_len(&summary_item->log_lines),
-						fwts_list_len(&summary_item->log_lines) > 1 ? "s" : "",
-						lines,
 						summary_item->text);
 				else
-					fwts_log_summary_verbatum(fw, " %s test, at %d log line%s: %s: %s",
+					fwts_log_summary_verbatum(fw, " %s: %s",
 						summary_item->test,
-						fwts_list_len(&summary_item->log_lines),
-						fwts_list_len(&summary_item->log_lines) > 1 ? "s" : "",
-						lines,
 						summary_item->text);
-
-				free(lines);
 			}
 			fwts_log_section_end(fw->results);
 		}