diff mbox

[3/3] lib: acpica + fwts_framework: add --acpica-debug option

Message ID 1370594499-25991-4-git-send-email-colin.king@canonical.com
State Accepted
Headers show

Commit Message

Colin Ian King June 7, 2013, 8:41 a.m. UTC 
From: Colin Ian King <colin.king@canonical.com>

fwts_acpica had some ACPICA debug helpers that were originally
intended to dump out ACPICA debug messages, but it was never
fully implemented.

Remove these helpers fwts_acpica_set_log_callback and
fwts_acpica_debug_command as well as the callback func pointer
fwts_acpica_log_callback_func as we can implement the debug
in a better way.

Re-work fwts_acpica_vprintf to now dump directly to the fwts
log if fw->flags has FWTS_FLAG_ACPICA_DEBUG set. Add the new
--acpica-debug option to the framework and documentation.

The --acpica-debug option is to help the fwts developers to see
the ACPICA internal warning and error messages rather than trying
to guess why fwts has caused ACPICA to misfunction.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 doc/fwts.1                       |  4 ++++
 src/acpica/fwts_acpica.c         | 39 +++++++++++++++------------------------
 src/lib/include/fwts_acpica.h    |  2 --
 src/lib/include/fwts_framework.h |  1 +
 src/lib/src/fwts_framework.c     |  4 ++++
 5 files changed, 24 insertions(+), 26 deletions(-)

Comments

Alex Hung June 10, 2013, 7:45 a.m. UTC | #1 
On 06/07/2013 04:41 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> fwts_acpica had some ACPICA debug helpers that were originally
> intended to dump out ACPICA debug messages, but it was never
> fully implemented.
>
> Remove these helpers fwts_acpica_set_log_callback and
> fwts_acpica_debug_command as well as the callback func pointer
> fwts_acpica_log_callback_func as we can implement the debug
> in a better way.
>
> Re-work fwts_acpica_vprintf to now dump directly to the fwts
> log if fw->flags has FWTS_FLAG_ACPICA_DEBUG set. Add the new
> --acpica-debug option to the framework and documentation.
>
> The --acpica-debug option is to help the fwts developers to see
> the ACPICA internal warning and error messages rather than trying
> to guess why fwts has caused ACPICA to misfunction.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   doc/fwts.1                       |  4 ++++
>   src/acpica/fwts_acpica.c         | 39 +++++++++++++++------------------------
>   src/lib/include/fwts_acpica.h    |  2 --
>   src/lib/include/fwts_framework.h |  1 +
>   src/lib/src/fwts_framework.c     |  4 ++++
>   5 files changed, 24 insertions(+), 26 deletions(-)
>
> diff --git a/doc/fwts.1 b/doc/fwts.1
> index b6ef362..187751c 100644
> --- a/doc/fwts.1
> +++ b/doc/fwts.1
> @@ -60,6 +60,10 @@ fwts options are as follow:
>   .B \-
>   output results to stdout.
>   .TP
> +.B \-\-acpica\-debug
> +enable ACPICA debug warning and error messages when invoking the ACPICA subsystem. This is mainly
> +for fwts developers to help track down any ACPICA interfacing issues with fwts.
> +.TP
>   .B \-a, \-\-all
>   run all the tests.
>   .TP
> diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c
> index 42b68f1..e396cd9 100644
> --- a/src/acpica/fwts_acpica.c
> +++ b/src/acpica/fwts_acpica.c
> @@ -17,6 +17,8 @@
>    *
>    */
>
> +#define _GNU_SOURCE
> +
>   #include <unistd.h>
>   #include <stdio.h>
>   #include <string.h>
> @@ -96,7 +98,6 @@ static void 			*fwts_acpica_DSDT;
>
>   static fwts_framework		*fwts_acpica_fw;		/* acpica context copy of fw */
>   static bool			fwts_acpica_init_called;	/* > 0, ACPICA initialised */
> -static fwts_acpica_log_callback fwts_acpica_log_callback_func = NULL;	/* logging call back func */
>
>   /* Semaphore Tracking */
>
> @@ -384,25 +385,6 @@ ACPI_PHYSICAL_ADDRESS AeLocalGetRootPointer(void)
>   }
>
>   /*
> - *  fwts_acpica_set_log_callback()
> - *	define logging callback function as used by fwts_acpica_vprintf()
> - */
> -void fwts_acpica_set_log_callback(fwts_framework *fw, fwts_acpica_log_callback func)
> -{
> -	fwts_acpica_log_callback_func = func;
> -}
> -
> -/*
> - *  fwts_acpica_debug_command()
> - *	run a debugging command, requires a logging callback function (or set to NULL for none).
> - */
> -void fwts_acpica_debug_command(fwts_framework *fw, fwts_acpica_log_callback func, char *command)
> -{
> -	fwts_acpica_set_log_callback(fw, func);
> -	AcpiDbCommandDispatch(command, NULL, NULL);
> -}
> -
> -/*
>    *  fwts_acpica_vprintf()
>    *	accumulate prints from ACPICA engine, emit them when we hit terminal '\n'
>    */
> @@ -411,10 +393,18 @@ void fwts_acpica_vprintf(const char *fmt, va_list args)
>   	static char *buffer;
>   	static size_t buffer_len;
>
> -	char tmp[4096];
> +	char *tmp;
>   	size_t tmp_len;
>
> -	vsnprintf(tmp, sizeof(tmp), fmt, args);
> +	/* Only emit messages if in ACPICA debug mode */
> +	if (!(fwts_acpica_fw->flags & FWTS_FLAG_ACPICA_DEBUG))
> +		return;
> +
> +	if (vasprintf(&tmp, fmt, args) < 0) {
> +		fwts_log_info(fwts_acpica_fw, "Out of memory allocating ACPICA printf buffer.");
> +		return;
> +	}
> +
>   	tmp_len = strlen(tmp);
>
>   	if (buffer_len == 0) {
> @@ -434,11 +424,12 @@ void fwts_acpica_vprintf(const char *fmt, va_list args)
>   	}
>
>   	if (index(buffer, '\n') != NULL) {
> -		if (fwts_acpica_log_callback_func)
> -			fwts_acpica_log_callback_func(fwts_acpica_fw, buffer);
> +		fwts_log_info(fwts_acpica_fw, "%s", buffer);
>   		free(buffer);
>   		buffer_len = 0;
>   	}
> +
> +	free(tmp);
>   }
>
>   /*
> diff --git a/src/lib/include/fwts_acpica.h b/src/lib/include/fwts_acpica.h
> index 17a5988..ab492e7 100644
> --- a/src/lib/include/fwts_acpica.h
> +++ b/src/lib/include/fwts_acpica.h
> @@ -26,8 +26,6 @@ typedef void (*fwts_acpica_log_callback)(fwts_framework *fw, const char *buffer)
>
>   int  fwts_acpica_init(fwts_framework *fw);
>   int  fwts_acpica_deinit(void);
> -void fwts_acpica_set_log_callback(fwts_framework *fw, fwts_acpica_log_callback func);
> -void fwts_acpica_debug_command(fwts_framework *fw, fwts_acpica_log_callback func, char *command);
>   fwts_list *fwts_acpica_get_object_names(int type);
>   void fwts_acpica_sem_count_clear(void);
>   void fwts_acpica_sem_count_get(int *acquired, int *released);
> diff --git a/src/lib/include/fwts_framework.h b/src/lib/include/fwts_framework.h
> index 7a540ad..ab831a9 100644
> --- a/src/lib/include/fwts_framework.h
> +++ b/src/lib/include/fwts_framework.h
> @@ -36,6 +36,7 @@ typedef enum {
>   	FWTS_FLAG_FORCE_CLEAN			= 0x00000004,
>   	FWTS_FLAG_SHOW_TESTS			= 0x00000008,
>   	FWTS_FLAG_SHOW_PROGRESS_DIALOG		= 0x00000010,
> +	FWTS_FLAG_ACPICA_DEBUG			= 0x00000020,
>   	FWTS_FLAG_BATCH				= 0x00001000,
>   	FWTS_FLAG_INTERACTIVE			= 0x00002000,
>   	FWTS_FLAG_BATCH_EXPERIMENTAL		= 0x00004000,
> diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c
> index ff1e3b3..1cf49fc 100644
> --- a/src/lib/src/fwts_framework.c
> +++ b/src/lib/src/fwts_framework.c
> @@ -82,6 +82,7 @@ static fwts_option fwts_framework_options[] = {
>   	{ "unsafe",		"U",  0, "Unsafe tests (tests that can potentially cause kernel oopses." },
>   	{ "filter-error-discard", "", 1, "Discard errors that match any of the specified labels." },
>   	{ "filter-error-keep",	"",   1, "Keep errors that match any of the specified labels." },
> +	{ "acpica-debug",	"",   0, "Enable ACPICA debug/warning messages." },
>   	{ NULL, NULL, 0, NULL }
>   };
>
> @@ -1099,6 +1100,9 @@ int fwts_framework_options_handler(fwts_framework *fw, int argc, char * const ar
>   			if (fwts_framework_filter_error_parse(optarg, &fw->errors_filter_keep) != FWTS_OK)
>   				return FWTS_ERROR;
>   			break;
> +		case 36: /* --acpica-debug */
> +			fw->flags |= FWTS_FLAG_ACPICA_DEBUG;
> +			break;
>   		}
>   		break;
>   	case 'a': /* --all */
>

Acked-by: <alex.hung@canonical.com>
Keng-Yu Lin June 19, 2013, 7:20 a.m. UTC | #2 
On Fri, Jun 7, 2013 at 4:41 PM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> fwts_acpica had some ACPICA debug helpers that were originally
> intended to dump out ACPICA debug messages, but it was never
> fully implemented.
>
> Remove these helpers fwts_acpica_set_log_callback and
> fwts_acpica_debug_command as well as the callback func pointer
> fwts_acpica_log_callback_func as we can implement the debug
> in a better way.
>
> Re-work fwts_acpica_vprintf to now dump directly to the fwts
> log if fw->flags has FWTS_FLAG_ACPICA_DEBUG set. Add the new
> --acpica-debug option to the framework and documentation.
>
> The --acpica-debug option is to help the fwts developers to see
> the ACPICA internal warning and error messages rather than trying
> to guess why fwts has caused ACPICA to misfunction.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  doc/fwts.1                       |  4 ++++
>  src/acpica/fwts_acpica.c         | 39 +++++++++++++++------------------------
>  src/lib/include/fwts_acpica.h    |  2 --
>  src/lib/include/fwts_framework.h |  1 +
>  src/lib/src/fwts_framework.c     |  4 ++++
>  5 files changed, 24 insertions(+), 26 deletions(-)
>
> diff --git a/doc/fwts.1 b/doc/fwts.1
> index b6ef362..187751c 100644
> --- a/doc/fwts.1
> +++ b/doc/fwts.1
> @@ -60,6 +60,10 @@ fwts options are as follow:
>  .B \-
>  output results to stdout.
>  .TP
> +.B \-\-acpica\-debug
> +enable ACPICA debug warning and error messages when invoking the ACPICA subsystem. This is mainly
> +for fwts developers to help track down any ACPICA interfacing issues with fwts.
> +.TP
>  .B \-a, \-\-all
>  run all the tests.
>  .TP
> diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c
> index 42b68f1..e396cd9 100644
> --- a/src/acpica/fwts_acpica.c
> +++ b/src/acpica/fwts_acpica.c
> @@ -17,6 +17,8 @@
>   *
>   */
>
> +#define _GNU_SOURCE
> +
>  #include <unistd.h>
>  #include <stdio.h>
>  #include <string.h>
> @@ -96,7 +98,6 @@ static void                   *fwts_acpica_DSDT;
>
>  static fwts_framework          *fwts_acpica_fw;                /* acpica context copy of fw */
>  static bool                    fwts_acpica_init_called;        /* > 0, ACPICA initialised */
> -static fwts_acpica_log_callback fwts_acpica_log_callback_func = NULL;  /* logging call back func */
>
>  /* Semaphore Tracking */
>
> @@ -384,25 +385,6 @@ ACPI_PHYSICAL_ADDRESS AeLocalGetRootPointer(void)
>  }
>
>  /*
> - *  fwts_acpica_set_log_callback()
> - *     define logging callback function as used by fwts_acpica_vprintf()
> - */
> -void fwts_acpica_set_log_callback(fwts_framework *fw, fwts_acpica_log_callback func)
> -{
> -       fwts_acpica_log_callback_func = func;
> -}
> -
> -/*
> - *  fwts_acpica_debug_command()
> - *     run a debugging command, requires a logging callback function (or set to NULL for none).
> - */
> -void fwts_acpica_debug_command(fwts_framework *fw, fwts_acpica_log_callback func, char *command)
> -{
> -       fwts_acpica_set_log_callback(fw, func);
> -       AcpiDbCommandDispatch(command, NULL, NULL);
> -}
> -
> -/*
>   *  fwts_acpica_vprintf()
>   *     accumulate prints from ACPICA engine, emit them when we hit terminal '\n'
>   */
> @@ -411,10 +393,18 @@ void fwts_acpica_vprintf(const char *fmt, va_list args)
>         static char *buffer;
>         static size_t buffer_len;
>
> -       char tmp[4096];
> +       char *tmp;
>         size_t tmp_len;
>
> -       vsnprintf(tmp, sizeof(tmp), fmt, args);
> +       /* Only emit messages if in ACPICA debug mode */
> +       if (!(fwts_acpica_fw->flags & FWTS_FLAG_ACPICA_DEBUG))
> +               return;
> +
> +       if (vasprintf(&tmp, fmt, args) < 0) {
> +               fwts_log_info(fwts_acpica_fw, "Out of memory allocating ACPICA printf buffer.");
> +               return;
> +       }
> +
>         tmp_len = strlen(tmp);
>
>         if (buffer_len == 0) {
> @@ -434,11 +424,12 @@ void fwts_acpica_vprintf(const char *fmt, va_list args)
>         }
>
>         if (index(buffer, '\n') != NULL) {
> -               if (fwts_acpica_log_callback_func)
> -                       fwts_acpica_log_callback_func(fwts_acpica_fw, buffer);
> +               fwts_log_info(fwts_acpica_fw, "%s", buffer);
>                 free(buffer);
>                 buffer_len = 0;
>         }
> +
> +       free(tmp);
>  }
>
>  /*
> diff --git a/src/lib/include/fwts_acpica.h b/src/lib/include/fwts_acpica.h
> index 17a5988..ab492e7 100644
> --- a/src/lib/include/fwts_acpica.h
> +++ b/src/lib/include/fwts_acpica.h
> @@ -26,8 +26,6 @@ typedef void (*fwts_acpica_log_callback)(fwts_framework *fw, const char *buffer)
>
>  int  fwts_acpica_init(fwts_framework *fw);
>  int  fwts_acpica_deinit(void);
> -void fwts_acpica_set_log_callback(fwts_framework *fw, fwts_acpica_log_callback func);
> -void fwts_acpica_debug_command(fwts_framework *fw, fwts_acpica_log_callback func, char *command);
>  fwts_list *fwts_acpica_get_object_names(int type);
>  void fwts_acpica_sem_count_clear(void);
>  void fwts_acpica_sem_count_get(int *acquired, int *released);
> diff --git a/src/lib/include/fwts_framework.h b/src/lib/include/fwts_framework.h
> index 7a540ad..ab831a9 100644
> --- a/src/lib/include/fwts_framework.h
> +++ b/src/lib/include/fwts_framework.h
> @@ -36,6 +36,7 @@ typedef enum {
>         FWTS_FLAG_FORCE_CLEAN                   = 0x00000004,
>         FWTS_FLAG_SHOW_TESTS                    = 0x00000008,
>         FWTS_FLAG_SHOW_PROGRESS_DIALOG          = 0x00000010,
> +       FWTS_FLAG_ACPICA_DEBUG                  = 0x00000020,
>         FWTS_FLAG_BATCH                         = 0x00001000,
>         FWTS_FLAG_INTERACTIVE                   = 0x00002000,
>         FWTS_FLAG_BATCH_EXPERIMENTAL            = 0x00004000,
> diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c
> index ff1e3b3..1cf49fc 100644
> --- a/src/lib/src/fwts_framework.c
> +++ b/src/lib/src/fwts_framework.c
> @@ -82,6 +82,7 @@ static fwts_option fwts_framework_options[] = {
>         { "unsafe",             "U",  0, "Unsafe tests (tests that can potentially cause kernel oopses." },
>         { "filter-error-discard", "", 1, "Discard errors that match any of the specified labels." },
>         { "filter-error-keep",  "",   1, "Keep errors that match any of the specified labels." },
> +       { "acpica-debug",       "",   0, "Enable ACPICA debug/warning messages." },
>         { NULL, NULL, 0, NULL }
>  };
>
> @@ -1099,6 +1100,9 @@ int fwts_framework_options_handler(fwts_framework *fw, int argc, char * const ar
>                         if (fwts_framework_filter_error_parse(optarg, &fw->errors_filter_keep) != FWTS_OK)
>                                 return FWTS_ERROR;
>                         break;
> +               case 36: /* --acpica-debug */
> +                       fw->flags |= FWTS_FLAG_ACPICA_DEBUG;
> +                       break;
>                 }
>                 break;
>         case 'a': /* --all */
> --
> 1.8.3
>

Acked-by: Keng-Yu Lin <kengyu@canonical.com>
diff mbox

Patch

diff --git a/doc/fwts.1 b/doc/fwts.1
index b6ef362..187751c 100644
--- a/doc/fwts.1
+++ b/doc/fwts.1
@@ -60,6 +60,10 @@  fwts options are as follow:
 .B \-
 output results to stdout.
 .TP
+.B \-\-acpica\-debug
+enable ACPICA debug warning and error messages when invoking the ACPICA subsystem. This is mainly
+for fwts developers to help track down any ACPICA interfacing issues with fwts.
+.TP
 .B \-a, \-\-all
 run all the tests.
 .TP
diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c
index 42b68f1..e396cd9 100644
--- a/src/acpica/fwts_acpica.c
+++ b/src/acpica/fwts_acpica.c
@@ -17,6 +17,8 @@ 
  *
  */
 
+#define _GNU_SOURCE
+
 #include <unistd.h>
 #include <stdio.h>
 #include <string.h>
@@ -96,7 +98,6 @@  static void 			*fwts_acpica_DSDT;
 
 static fwts_framework		*fwts_acpica_fw;		/* acpica context copy of fw */
 static bool			fwts_acpica_init_called;	/* > 0, ACPICA initialised */
-static fwts_acpica_log_callback fwts_acpica_log_callback_func = NULL;	/* logging call back func */
 
 /* Semaphore Tracking */
 
@@ -384,25 +385,6 @@  ACPI_PHYSICAL_ADDRESS AeLocalGetRootPointer(void)
 }
 
 /*
- *  fwts_acpica_set_log_callback()
- *	define logging callback function as used by fwts_acpica_vprintf()
- */
-void fwts_acpica_set_log_callback(fwts_framework *fw, fwts_acpica_log_callback func)
-{
-	fwts_acpica_log_callback_func = func;
-}
-
-/*
- *  fwts_acpica_debug_command()
- *	run a debugging command, requires a logging callback function (or set to NULL for none).
- */
-void fwts_acpica_debug_command(fwts_framework *fw, fwts_acpica_log_callback func, char *command)
-{
-	fwts_acpica_set_log_callback(fw, func);
-	AcpiDbCommandDispatch(command, NULL, NULL);
-}
-
-/*
  *  fwts_acpica_vprintf()
  *	accumulate prints from ACPICA engine, emit them when we hit terminal '\n'
  */
@@ -411,10 +393,18 @@  void fwts_acpica_vprintf(const char *fmt, va_list args)
 	static char *buffer;
 	static size_t buffer_len;
 
-	char tmp[4096];
+	char *tmp;
 	size_t tmp_len;
 
-	vsnprintf(tmp, sizeof(tmp), fmt, args);
+	/* Only emit messages if in ACPICA debug mode */
+	if (!(fwts_acpica_fw->flags & FWTS_FLAG_ACPICA_DEBUG))
+		return;
+
+	if (vasprintf(&tmp, fmt, args) < 0) {
+		fwts_log_info(fwts_acpica_fw, "Out of memory allocating ACPICA printf buffer.");
+		return;
+	}
+
 	tmp_len = strlen(tmp);
 
 	if (buffer_len == 0) {
@@ -434,11 +424,12 @@  void fwts_acpica_vprintf(const char *fmt, va_list args)
 	}
 
 	if (index(buffer, '\n') != NULL) {
-		if (fwts_acpica_log_callback_func)
-			fwts_acpica_log_callback_func(fwts_acpica_fw, buffer);
+		fwts_log_info(fwts_acpica_fw, "%s", buffer);
 		free(buffer);
 		buffer_len = 0;
 	}
+
+	free(tmp);
 }
 
 /*
diff --git a/src/lib/include/fwts_acpica.h b/src/lib/include/fwts_acpica.h
index 17a5988..ab492e7 100644
--- a/src/lib/include/fwts_acpica.h
+++ b/src/lib/include/fwts_acpica.h
@@ -26,8 +26,6 @@  typedef void (*fwts_acpica_log_callback)(fwts_framework *fw, const char *buffer)
 
 int  fwts_acpica_init(fwts_framework *fw);
 int  fwts_acpica_deinit(void);
-void fwts_acpica_set_log_callback(fwts_framework *fw, fwts_acpica_log_callback func);
-void fwts_acpica_debug_command(fwts_framework *fw, fwts_acpica_log_callback func, char *command);
 fwts_list *fwts_acpica_get_object_names(int type);
 void fwts_acpica_sem_count_clear(void);
 void fwts_acpica_sem_count_get(int *acquired, int *released);
diff --git a/src/lib/include/fwts_framework.h b/src/lib/include/fwts_framework.h
index 7a540ad..ab831a9 100644
--- a/src/lib/include/fwts_framework.h
+++ b/src/lib/include/fwts_framework.h
@@ -36,6 +36,7 @@  typedef enum {
 	FWTS_FLAG_FORCE_CLEAN			= 0x00000004,
 	FWTS_FLAG_SHOW_TESTS			= 0x00000008,
 	FWTS_FLAG_SHOW_PROGRESS_DIALOG		= 0x00000010,
+	FWTS_FLAG_ACPICA_DEBUG			= 0x00000020,
 	FWTS_FLAG_BATCH				= 0x00001000,
 	FWTS_FLAG_INTERACTIVE			= 0x00002000,
 	FWTS_FLAG_BATCH_EXPERIMENTAL		= 0x00004000,
diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c
index ff1e3b3..1cf49fc 100644
--- a/src/lib/src/fwts_framework.c
+++ b/src/lib/src/fwts_framework.c
@@ -82,6 +82,7 @@  static fwts_option fwts_framework_options[] = {
 	{ "unsafe",		"U",  0, "Unsafe tests (tests that can potentially cause kernel oopses." },
 	{ "filter-error-discard", "", 1, "Discard errors that match any of the specified labels." },
 	{ "filter-error-keep",	"",   1, "Keep errors that match any of the specified labels." },
+	{ "acpica-debug",	"",   0, "Enable ACPICA debug/warning messages." },
 	{ NULL, NULL, 0, NULL }
 };
 
@@ -1099,6 +1100,9 @@  int fwts_framework_options_handler(fwts_framework *fw, int argc, char * const ar
 			if (fwts_framework_filter_error_parse(optarg, &fw->errors_filter_keep) != FWTS_OK)
 				return FWTS_ERROR;
 			break;
+		case 36: /* --acpica-debug */
+			fw->flags |= FWTS_FLAG_ACPICA_DEBUG;
+			break;
 		}
 		break;
 	case 'a': /* --all */