Patchwork lib + tests: modify fwts_pipe_exec to return the child exit status

login
register
mail settings
Submitter Colin King
Date Dec. 7, 2012, 2:08 p.m.
Message ID <1354889329-30567-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/204494/
State Accepted
Headers show

Comments

Colin King - Dec. 7, 2012, 2:08 p.m.
From: Colin Ian King <colin.king@canonical.com>

Fix bug that has been in the fwts s3, s4 and s3power tests. Originally the
semantics to fwts_pipe_exec() was that was meant to return the child exit status
but this got changed to return FWTS_OK if successful and FWTS_EXEC_FAILED if
the child exec failed.  Thus we no longer passed back the child exit status which
the s3, s4 and s4power tests relied on to check the suspend or hibernate pm script
exit status.

This patch adds an extra child exit status parameter to fwts_pipe_exec() which can
be checked if required.  However, for most use cases, one can ignore this and just
check for FWTS_OK or FWTS_EXEC_FAILED if you don't care about the child exit status.

The patch fixes up all the calls to fwts_pipe_exec and also changes the way errors
are detected assuming anything that is not a FWTS_OK return is a failure.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/acpi/mcfg/mcfg.c            |  3 ++-
 src/acpi/s3/s3.c                |  2 +-
 src/acpi/s3power/s3power.c      |  3 +--
 src/acpi/s4/s4.c                |  2 +-
 src/bios/mtrr/mtrr.c            |  3 ++-
 src/hotkey/hotkey/hotkey.c      |  3 ++-
 src/lib/include/fwts_pipeio.h   |  2 +-
 src/lib/src/fwts_efi_module.c   |  8 ++++++--
 src/lib/src/fwts_hwinfo.c       | 20 +++++++++++---------
 src/lib/src/fwts_pipeio.c       | 12 ++++++------
 src/pci/aspm/aspm.c             |  6 ++++--
 src/pci/maxreadreq/maxreadreq.c |  4 +++-
 12 files changed, 40 insertions(+), 28 deletions(-)
Keng-Yu Lin - Dec. 12, 2012, 3:14 a.m.
On Fri, Dec 7, 2012 at 10:08 PM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Fix bug that has been in the fwts s3, s4 and s3power tests. Originally the
> semantics to fwts_pipe_exec() was that was meant to return the child exit status
> but this got changed to return FWTS_OK if successful and FWTS_EXEC_FAILED if
> the child exec failed.  Thus we no longer passed back the child exit status which
> the s3, s4 and s4power tests relied on to check the suspend or hibernate pm script
> exit status.
>
> This patch adds an extra child exit status parameter to fwts_pipe_exec() which can
> be checked if required.  However, for most use cases, one can ignore this and just
> check for FWTS_OK or FWTS_EXEC_FAILED if you don't care about the child exit status.
>
> The patch fixes up all the calls to fwts_pipe_exec and also changes the way errors
> are detected assuming anything that is not a FWTS_OK return is a failure.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/acpi/mcfg/mcfg.c            |  3 ++-
>  src/acpi/s3/s3.c                |  2 +-
>  src/acpi/s3power/s3power.c      |  3 +--
>  src/acpi/s4/s4.c                |  2 +-
>  src/bios/mtrr/mtrr.c            |  3 ++-
>  src/hotkey/hotkey/hotkey.c      |  3 ++-
>  src/lib/include/fwts_pipeio.h   |  2 +-
>  src/lib/src/fwts_efi_module.c   |  8 ++++++--
>  src/lib/src/fwts_hwinfo.c       | 20 +++++++++++---------
>  src/lib/src/fwts_pipeio.c       | 12 ++++++------
>  src/pci/aspm/aspm.c             |  6 ++++--
>  src/pci/maxreadreq/maxreadreq.c |  4 +++-
>  12 files changed, 40 insertions(+), 28 deletions(-)
>
> diff --git a/src/acpi/mcfg/mcfg.c b/src/acpi/mcfg/mcfg.c
> index 331c24f..39ae697 100644
> --- a/src/acpi/mcfg/mcfg.c
> +++ b/src/acpi/mcfg/mcfg.c
> @@ -40,6 +40,7 @@ static void compare_config_space(
>  {
>         fwts_list *lspci_output;
>         fwts_list_link *item;
> +       int status;
>
>         char command[PATH_MAX];
>         char compare_line[50];
> @@ -54,7 +55,7 @@ static void compare_config_space(
>         snprintf(command, sizeof(command), "%s -vxxx -s %" PRIu16 ":%" PRIu32,
>                 fw->lspci, segment, device);
>
> -       if (fwts_pipe_exec(command, &lspci_output) == FWTS_EXEC_ERROR) {
> +       if (fwts_pipe_exec(command, &lspci_output, &status) != FWTS_OK) {
>                 fwts_log_warning(fw, "Could not execute %s", command);
>                 return;
>         }
> diff --git a/src/acpi/s3/s3.c b/src/acpi/s3/s3.c
> index a1b7848..182db43 100644
> --- a/src/acpi/s3/s3.c
> +++ b/src/acpi/s3/s3.c
> @@ -106,7 +106,7 @@ static int s3_do_suspend_resume(fwts_framework *fw,
>         /* Do S3 here */
>         fwts_progress_message(fw, percent, "(Suspending)");
>         time(&t_start);
> -       status = fwts_pipe_exec(command, &output);
> +       (void)fwts_pipe_exec(command, &output, &status);
>         time(&t_end);
>         fwts_progress_message(fw, percent, "(Resumed)");
>         fwts_text_list_free(output);
> diff --git a/src/acpi/s3power/s3power.c b/src/acpi/s3power/s3power.c
> index a8a7c25..0b98df1 100644
> --- a/src/acpi/s3power/s3power.c
> +++ b/src/acpi/s3power/s3power.c
> @@ -191,8 +191,7 @@ static int s3power_test(fwts_framework *fw)
>         /* Do S3 here */
>         fwts_progress_message(fw, 100, "(Suspending)");
>         time(&t_start);
> -       status = 0;
> -       status = fwts_pipe_exec(PM_SUSPEND, &output);
> +       (void)fwts_pipe_exec(PM_SUSPEND, &output, &status);
>         time(&t_end);
>         fwts_progress_message(fw, 100, "(Resumed)");
>         fwts_text_list_free(output);
> diff --git a/src/acpi/s4/s4.c b/src/acpi/s4/s4.c
> index 728062d..bea7fce 100644
> --- a/src/acpi/s4/s4.c
> +++ b/src/acpi/s4/s4.c
> @@ -137,7 +137,7 @@ static int s4_hibernate(fwts_framework *fw,
>
>         /* Do s4 here */
>         fwts_progress_message(fw, percent, "(Hibernating)");
> -       status = fwts_pipe_exec(command, &output);
> +       (void)fwts_pipe_exec(command, &output, &status);
>         fwts_progress_message(fw, percent, "(Resumed)");
>         fwts_text_list_free(output);
>         free(command);
> diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c
> index 6dbc8e2..62a91c7 100644
> --- a/src/bios/mtrr/mtrr.c
> +++ b/src/bios/mtrr/mtrr.c
> @@ -199,11 +199,12 @@ static bool is_prefetchable(fwts_framework *fw, char *device, uint64_t address)
>         char line[4096];
>         fwts_list *lspci_output;
>         fwts_list_link *item;
> +       int status;
>
>         memset(line,0,4096);
>
>         snprintf(line, sizeof(line), "%s -v -s %s", fw->lspci, device);
> -       fwts_pipe_exec(line, &lspci_output);
> +       fwts_pipe_exec(line, &lspci_output, &status);
>         if (lspci_output == NULL)
>                 return pref;
>
> diff --git a/src/hotkey/hotkey/hotkey.c b/src/hotkey/hotkey/hotkey.c
> index 267b69e..59a6829 100644
> --- a/src/hotkey/hotkey/hotkey.c
> +++ b/src/hotkey/hotkey/hotkey.c
> @@ -182,9 +182,10 @@ static char *hotkey_find_keymap(char *device)
>
>         char buffer[1024];
>         char *keymap = NULL;
> +       int status;
>
>         snprintf(buffer, sizeof(buffer), "udevadm test /class/%s 2>&1", device);
> -       if (fwts_pipe_exec(buffer, &output) != FWTS_OK)
> +       if (fwts_pipe_exec(buffer, &output, &status) != FWTS_OK)
>                 return NULL;
>
>         snprintf(buffer, sizeof(buffer), "keymap %s", device);
> diff --git a/src/lib/include/fwts_pipeio.h b/src/lib/include/fwts_pipeio.h
> index f1875da..04f4dea 100644
> --- a/src/lib/include/fwts_pipeio.h
> +++ b/src/lib/include/fwts_pipeio.h
> @@ -31,6 +31,6 @@
>  int   fwts_pipe_open(const char *command, pid_t *childpid);
>  char *fwts_pipe_read(const int fd, ssize_t *length);
>  int   fwts_pipe_close(const int fd, const pid_t pid);
> -int   fwts_pipe_exec(const char *command, fwts_list **list);
> +int   fwts_pipe_exec(const char *command, fwts_list **list, int *status);
>
>  #endif
> diff --git a/src/lib/src/fwts_efi_module.c b/src/lib/src/fwts_efi_module.c
> index 26e5740..c92a091 100644
> --- a/src/lib/src/fwts_efi_module.c
> +++ b/src/lib/src/fwts_efi_module.c
> @@ -57,7 +57,9 @@ int fwts_lib_efi_runtime_load_module(fwts_framework *fw)
>         }
>
>         if (!module_already_loaded) {
> -               if (fwts_pipe_exec("modprobe efi_runtime", &output) != FWTS_OK) {
> +               int status;
> +
> +               if (fwts_pipe_exec("modprobe efi_runtime", &output, &status) != FWTS_OK) {
>                         fwts_log_error(fw, "Load efi_runtime module error.");
>                         return FWTS_ERROR;
>                 } else {
> @@ -94,7 +96,9 @@ int fwts_lib_efi_runtime_unload_module(fwts_framework *fw)
>                 return FWTS_ERROR;
>         }
>         if (module_already_loaded) {
> -               if (fwts_pipe_exec("modprobe -r efi_runtime", &output) != FWTS_OK) {
> +               int status;
> +
> +               if (fwts_pipe_exec("modprobe -r efi_runtime", &output, &status) != FWTS_OK) {
>                         fwts_log_error(fw, "Unload efi_runtime module error.");
>                         return FWTS_ERROR;
>                 } else {
> diff --git a/src/lib/src/fwts_hwinfo.c b/src/lib/src/fwts_hwinfo.c
> index 4365c64..3fcac80 100644
> --- a/src/lib/src/fwts_hwinfo.c
> +++ b/src/lib/src/fwts_hwinfo.c
> @@ -33,15 +33,17 @@ int fwts_hwinfo_get(fwts_framework *fw, fwts_hwinfo *hwinfo)
>  {
>         FWTS_UNUSED(fw);
>
> -       fwts_pipe_exec("lspci | grep Network", &hwinfo->network);
> -       fwts_pipe_exec("lspci | grep Ethernet", &hwinfo->ethernet);
> -       fwts_pipe_exec("ifconfig -a | grep -A1 '^\\w'", &hwinfo->ifconfig);
> -       fwts_pipe_exec("iwconfig | grep -A1 '^\\w'", &hwinfo->iwconfig);
> -       fwts_pipe_exec("hciconfig -a | grep -A2 '^\\w", &hwinfo->hciconfig);
> -       fwts_pipe_exec("lspci | grep VGA", &hwinfo->videocard);
> -       fwts_pipe_exec("xinput list", &hwinfo->xinput);
> -       fwts_pipe_exec("pactl list | grep Sink | grep -v Latency", &hwinfo->pactl);
> -       fwts_pipe_exec("pactl list | grep Source | grep -v Latency", &hwinfo->pactl);
> +       int status;
> +
> +       fwts_pipe_exec("lspci | grep Network", &hwinfo->network, &status);
> +       fwts_pipe_exec("lspci | grep Ethernet", &hwinfo->ethernet, &status);
> +       fwts_pipe_exec("ifconfig -a | grep -A1 '^\\w'", &hwinfo->ifconfig, &status);
> +       fwts_pipe_exec("iwconfig | grep -A1 '^\\w'", &hwinfo->iwconfig, &status);
> +       fwts_pipe_exec("hciconfig -a | grep -A2 '^\\w", &hwinfo->hciconfig, &status);
> +       fwts_pipe_exec("lspci | grep VGA", &hwinfo->videocard, &status);
> +       fwts_pipe_exec("xinput list", &hwinfo->xinput, &status);
> +       fwts_pipe_exec("pactl list | grep Sink | grep -v Latency", &hwinfo->pactl, &status);
> +       fwts_pipe_exec("pactl list | grep Source | grep -v Latency", &hwinfo->pactl, &status);
>
>         return FWTS_OK;
>  }
> diff --git a/src/lib/src/fwts_pipeio.c b/src/lib/src/fwts_pipeio.c
> index 68abee9..5af817b 100644
> --- a/src/lib/src/fwts_pipeio.c
> +++ b/src/lib/src/fwts_pipeio.c
> @@ -133,14 +133,15 @@ int fwts_pipe_close(const int fd, const pid_t pid)
>   *  fwts_pipe_exec()
>   *     execute a command, return a list containing lines
>   *     of the stdout output from the command.
> + *     Return FWTS_OK if the exec worked, FWTS_EXEC_ERROR if
> + *     it failed.  status contains the child exit status.
>   */
> -int fwts_pipe_exec(const char *command, fwts_list **list)
> +int fwts_pipe_exec(const char *command, fwts_list **list, int *status)
>  {
>         pid_t   pid;
>         int     fd;
>         ssize_t len;
>         char    *text;
> -       int     ret;
>
>         if ((fd = fwts_pipe_open(command, &pid)) < 0)
>                 return FWTS_ERROR;
> @@ -149,12 +150,11 @@ int fwts_pipe_exec(const char *command, fwts_list **list)
>         *list = fwts_list_from_text(text);
>         free(text);
>
> -       ret = fwts_pipe_close(fd, pid);
> -
> -       if (ret == FWTS_EXEC_ERROR) {
> +       *status = fwts_pipe_close(fd, pid);
> +       if (*status) {
>                 fwts_list_free(*list, free);
>                 *list = NULL;
> -               return FWTS_ERROR;
> +               return FWTS_EXEC_ERROR;
>         }
>         return FWTS_OK;
>  }
> diff --git a/src/pci/aspm/aspm.c b/src/pci/aspm/aspm.c
> index 1a6cc27..a2cda8c 100644
> --- a/src/pci/aspm/aspm.c
> +++ b/src/pci/aspm/aspm.c
> @@ -229,10 +229,11 @@ static int pcie_check_aspm_registers(fwts_framework *fw)
>         struct pci_device *head = NULL, *cur = NULL, *device = NULL;
>         char command[PATH_MAX];
>         int ret = FWTS_OK;
> +       int status;
>
>         snprintf(command, sizeof(command), "%s", fw->lspci);
>
> -       if (fwts_pipe_exec(command, &lspci_output) == FWTS_EXEC_ERROR) {
> +       if (fwts_pipe_exec(command, &lspci_output, &status) != FWTS_OK) {
>                 fwts_log_warning(fw, "Could not execute %s", command);
>                 return FWTS_ERROR;
>         }
> @@ -267,10 +268,11 @@ static int pcie_check_aspm_registers(fwts_framework *fw)
>         for (cur = head; cur; cur = cur->next) {
>                 int reg_loc = 0, reg_val = 0;
>                 int i;
> +               int status;
>
>                 snprintf(command, sizeof(command), "%s -s %02X:%02X.%02X -xxx",
>                         fw->lspci, cur->bus, cur->dev, cur->func);
> -               if (fwts_pipe_exec(command, &lspci_output) == FWTS_EXEC_ERROR) {
> +               if (fwts_pipe_exec(command, &lspci_output, &status) != FWTS_OK) {
>                         fwts_log_warning(fw, "Could not execute %s", command);
>                         pci_device_list_free(head);
>                         return FWTS_ERROR;
> diff --git a/src/pci/maxreadreq/maxreadreq.c b/src/pci/maxreadreq/maxreadreq.c
> index e5ace46..92af83f 100644
> --- a/src/pci/maxreadreq/maxreadreq.c
> +++ b/src/pci/maxreadreq/maxreadreq.c
> @@ -37,10 +37,12 @@ static fwts_list *lspci_text;
>
>  static int maxreadreq_init(fwts_framework *fw)
>  {
> +       int status;
> +
>         if (fwts_check_executable(fw, fw->lspci, "lspci"))
>                 return FWTS_ERROR;
>
> -       if (fwts_pipe_exec("lspci -vvv", &lspci_text)) {
> +       if (fwts_pipe_exec("lspci -vvv", &lspci_text, &status) != FWTS_OK) {
>                 fwts_log_error(fw, "Failed to execute lspci -vvv.");
>                 return FWTS_ERROR;
>         }
> --
> 1.8.0
>
Acked-by: Keng-Yu Lin <kengyu@canonical.com>
Alex Hung - Dec. 12, 2012, 7:44 a.m.
On 12/07/2012 10:08 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Fix bug that has been in the fwts s3, s4 and s3power tests. Originally the
> semantics to fwts_pipe_exec() was that was meant to return the child exit status
> but this got changed to return FWTS_OK if successful and FWTS_EXEC_FAILED if
> the child exec failed.  Thus we no longer passed back the child exit status which
> the s3, s4 and s4power tests relied on to check the suspend or hibernate pm script
> exit status.
>
> This patch adds an extra child exit status parameter to fwts_pipe_exec() which can
> be checked if required.  However, for most use cases, one can ignore this and just
> check for FWTS_OK or FWTS_EXEC_FAILED if you don't care about the child exit status.
>
> The patch fixes up all the calls to fwts_pipe_exec and also changes the way errors
> are detected assuming anything that is not a FWTS_OK return is a failure.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/acpi/mcfg/mcfg.c            |  3 ++-
>   src/acpi/s3/s3.c                |  2 +-
>   src/acpi/s3power/s3power.c      |  3 +--
>   src/acpi/s4/s4.c                |  2 +-
>   src/bios/mtrr/mtrr.c            |  3 ++-
>   src/hotkey/hotkey/hotkey.c      |  3 ++-
>   src/lib/include/fwts_pipeio.h   |  2 +-
>   src/lib/src/fwts_efi_module.c   |  8 ++++++--
>   src/lib/src/fwts_hwinfo.c       | 20 +++++++++++---------
>   src/lib/src/fwts_pipeio.c       | 12 ++++++------
>   src/pci/aspm/aspm.c             |  6 ++++--
>   src/pci/maxreadreq/maxreadreq.c |  4 +++-
>   12 files changed, 40 insertions(+), 28 deletions(-)
>
> diff --git a/src/acpi/mcfg/mcfg.c b/src/acpi/mcfg/mcfg.c
> index 331c24f..39ae697 100644
> --- a/src/acpi/mcfg/mcfg.c
> +++ b/src/acpi/mcfg/mcfg.c
> @@ -40,6 +40,7 @@ static void compare_config_space(
>   {
>   	fwts_list *lspci_output;
>   	fwts_list_link *item;
> +	int status;
>
>   	char command[PATH_MAX];
>   	char compare_line[50];
> @@ -54,7 +55,7 @@ static void compare_config_space(
>   	snprintf(command, sizeof(command), "%s -vxxx -s %" PRIu16 ":%" PRIu32,
>   		fw->lspci, segment, device);
>
> -	if (fwts_pipe_exec(command, &lspci_output) == FWTS_EXEC_ERROR) {
> +	if (fwts_pipe_exec(command, &lspci_output, &status) != FWTS_OK) {
>   		fwts_log_warning(fw, "Could not execute %s", command);
>   		return;
>   	}
> diff --git a/src/acpi/s3/s3.c b/src/acpi/s3/s3.c
> index a1b7848..182db43 100644
> --- a/src/acpi/s3/s3.c
> +++ b/src/acpi/s3/s3.c
> @@ -106,7 +106,7 @@ static int s3_do_suspend_resume(fwts_framework *fw,
>   	/* Do S3 here */
>   	fwts_progress_message(fw, percent, "(Suspending)");
>   	time(&t_start);
> -	status = fwts_pipe_exec(command, &output);
> +	(void)fwts_pipe_exec(command, &output, &status);
>   	time(&t_end);
>   	fwts_progress_message(fw, percent, "(Resumed)");
>   	fwts_text_list_free(output);
> diff --git a/src/acpi/s3power/s3power.c b/src/acpi/s3power/s3power.c
> index a8a7c25..0b98df1 100644
> --- a/src/acpi/s3power/s3power.c
> +++ b/src/acpi/s3power/s3power.c
> @@ -191,8 +191,7 @@ static int s3power_test(fwts_framework *fw)
>   	/* Do S3 here */
>   	fwts_progress_message(fw, 100, "(Suspending)");
>   	time(&t_start);
> -	status = 0;
> -	status = fwts_pipe_exec(PM_SUSPEND, &output);
> +	(void)fwts_pipe_exec(PM_SUSPEND, &output, &status);
>   	time(&t_end);
>   	fwts_progress_message(fw, 100, "(Resumed)");
>   	fwts_text_list_free(output);
> diff --git a/src/acpi/s4/s4.c b/src/acpi/s4/s4.c
> index 728062d..bea7fce 100644
> --- a/src/acpi/s4/s4.c
> +++ b/src/acpi/s4/s4.c
> @@ -137,7 +137,7 @@ static int s4_hibernate(fwts_framework *fw,
>
>   	/* Do s4 here */
>   	fwts_progress_message(fw, percent, "(Hibernating)");
> -	status = fwts_pipe_exec(command, &output);
> +	(void)fwts_pipe_exec(command, &output, &status);
>   	fwts_progress_message(fw, percent, "(Resumed)");
>   	fwts_text_list_free(output);
>   	free(command);
> diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c
> index 6dbc8e2..62a91c7 100644
> --- a/src/bios/mtrr/mtrr.c
> +++ b/src/bios/mtrr/mtrr.c
> @@ -199,11 +199,12 @@ static bool is_prefetchable(fwts_framework *fw, char *device, uint64_t address)
>   	char line[4096];
>   	fwts_list *lspci_output;
>   	fwts_list_link *item;
> +	int status;
>
>   	memset(line,0,4096);
>
>   	snprintf(line, sizeof(line), "%s -v -s %s", fw->lspci, device);
> -	fwts_pipe_exec(line, &lspci_output);
> +	fwts_pipe_exec(line, &lspci_output, &status);
>   	if (lspci_output == NULL)
>   		return pref;
>
> diff --git a/src/hotkey/hotkey/hotkey.c b/src/hotkey/hotkey/hotkey.c
> index 267b69e..59a6829 100644
> --- a/src/hotkey/hotkey/hotkey.c
> +++ b/src/hotkey/hotkey/hotkey.c
> @@ -182,9 +182,10 @@ static char *hotkey_find_keymap(char *device)
>
>   	char buffer[1024];
>   	char *keymap = NULL;
> +	int status;
>
>   	snprintf(buffer, sizeof(buffer), "udevadm test /class/%s 2>&1", device);
> -	if (fwts_pipe_exec(buffer, &output) != FWTS_OK)
> +	if (fwts_pipe_exec(buffer, &output, &status) != FWTS_OK)
>   		return NULL;
>
>   	snprintf(buffer, sizeof(buffer), "keymap %s", device);
> diff --git a/src/lib/include/fwts_pipeio.h b/src/lib/include/fwts_pipeio.h
> index f1875da..04f4dea 100644
> --- a/src/lib/include/fwts_pipeio.h
> +++ b/src/lib/include/fwts_pipeio.h
> @@ -31,6 +31,6 @@
>   int   fwts_pipe_open(const char *command, pid_t *childpid);
>   char *fwts_pipe_read(const int fd, ssize_t *length);
>   int   fwts_pipe_close(const int fd, const pid_t pid);
> -int   fwts_pipe_exec(const char *command, fwts_list **list);
> +int   fwts_pipe_exec(const char *command, fwts_list **list, int *status);
>
>   #endif
> diff --git a/src/lib/src/fwts_efi_module.c b/src/lib/src/fwts_efi_module.c
> index 26e5740..c92a091 100644
> --- a/src/lib/src/fwts_efi_module.c
> +++ b/src/lib/src/fwts_efi_module.c
> @@ -57,7 +57,9 @@ int fwts_lib_efi_runtime_load_module(fwts_framework *fw)
>   	}
>
>   	if (!module_already_loaded) {
> -		if (fwts_pipe_exec("modprobe efi_runtime", &output) != FWTS_OK) {
> +		int status;
> +
> +		if (fwts_pipe_exec("modprobe efi_runtime", &output, &status) != FWTS_OK) {
>   			fwts_log_error(fw, "Load efi_runtime module error.");
>   			return FWTS_ERROR;
>   		} else {
> @@ -94,7 +96,9 @@ int fwts_lib_efi_runtime_unload_module(fwts_framework *fw)
>   		return FWTS_ERROR;
>   	}
>   	if (module_already_loaded) {
> -		if (fwts_pipe_exec("modprobe -r efi_runtime", &output) != FWTS_OK) {
> +		int status;
> +
> +		if (fwts_pipe_exec("modprobe -r efi_runtime", &output, &status) != FWTS_OK) {
>   			fwts_log_error(fw, "Unload efi_runtime module error.");
>   			return FWTS_ERROR;
>   		} else {
> diff --git a/src/lib/src/fwts_hwinfo.c b/src/lib/src/fwts_hwinfo.c
> index 4365c64..3fcac80 100644
> --- a/src/lib/src/fwts_hwinfo.c
> +++ b/src/lib/src/fwts_hwinfo.c
> @@ -33,15 +33,17 @@ int fwts_hwinfo_get(fwts_framework *fw, fwts_hwinfo *hwinfo)
>   {
>   	FWTS_UNUSED(fw);
>
> -	fwts_pipe_exec("lspci | grep Network", &hwinfo->network);
> -	fwts_pipe_exec("lspci | grep Ethernet", &hwinfo->ethernet);
> -	fwts_pipe_exec("ifconfig -a | grep -A1 '^\\w'", &hwinfo->ifconfig);
> -	fwts_pipe_exec("iwconfig | grep -A1 '^\\w'", &hwinfo->iwconfig);
> -	fwts_pipe_exec("hciconfig -a | grep -A2 '^\\w", &hwinfo->hciconfig);
> -	fwts_pipe_exec("lspci | grep VGA", &hwinfo->videocard);
> -	fwts_pipe_exec("xinput list", &hwinfo->xinput);
> -	fwts_pipe_exec("pactl list | grep Sink | grep -v Latency", &hwinfo->pactl);
> -	fwts_pipe_exec("pactl list | grep Source | grep -v Latency", &hwinfo->pactl);
> +	int status;
> +
> +	fwts_pipe_exec("lspci | grep Network", &hwinfo->network, &status);
> +	fwts_pipe_exec("lspci | grep Ethernet", &hwinfo->ethernet, &status);
> +	fwts_pipe_exec("ifconfig -a | grep -A1 '^\\w'", &hwinfo->ifconfig, &status);
> +	fwts_pipe_exec("iwconfig | grep -A1 '^\\w'", &hwinfo->iwconfig, &status);
> +	fwts_pipe_exec("hciconfig -a | grep -A2 '^\\w", &hwinfo->hciconfig, &status);
> +	fwts_pipe_exec("lspci | grep VGA", &hwinfo->videocard, &status);
> +	fwts_pipe_exec("xinput list", &hwinfo->xinput, &status);
> +	fwts_pipe_exec("pactl list | grep Sink | grep -v Latency", &hwinfo->pactl, &status);
> +	fwts_pipe_exec("pactl list | grep Source | grep -v Latency", &hwinfo->pactl, &status);
>
>   	return FWTS_OK;
>   }
> diff --git a/src/lib/src/fwts_pipeio.c b/src/lib/src/fwts_pipeio.c
> index 68abee9..5af817b 100644
> --- a/src/lib/src/fwts_pipeio.c
> +++ b/src/lib/src/fwts_pipeio.c
> @@ -133,14 +133,15 @@ int fwts_pipe_close(const int fd, const pid_t pid)
>    *  fwts_pipe_exec()
>    *	execute a command, return a list containing lines
>    *	of the stdout output from the command.
> + *	Return FWTS_OK if the exec worked, FWTS_EXEC_ERROR if
> + *	it failed.  status contains the child exit status.
>    */
> -int fwts_pipe_exec(const char *command, fwts_list **list)
> +int fwts_pipe_exec(const char *command, fwts_list **list, int *status)
>   {
>   	pid_t 	pid;
>   	int	fd;
>   	ssize_t	len;
>   	char 	*text;
> -	int	ret;
>
>   	if ((fd = fwts_pipe_open(command, &pid)) < 0)
>   		return FWTS_ERROR;
> @@ -149,12 +150,11 @@ int fwts_pipe_exec(const char *command, fwts_list **list)
>   	*list = fwts_list_from_text(text);
>   	free(text);
>
> -	ret = fwts_pipe_close(fd, pid);
> -
> -	if (ret == FWTS_EXEC_ERROR) {
> +	*status = fwts_pipe_close(fd, pid);
> +	if (*status) {
>   		fwts_list_free(*list, free);
>   		*list = NULL;
> -		return FWTS_ERROR;
> +		return FWTS_EXEC_ERROR;
>   	}
>   	return FWTS_OK;
>   }
> diff --git a/src/pci/aspm/aspm.c b/src/pci/aspm/aspm.c
> index 1a6cc27..a2cda8c 100644
> --- a/src/pci/aspm/aspm.c
> +++ b/src/pci/aspm/aspm.c
> @@ -229,10 +229,11 @@ static int pcie_check_aspm_registers(fwts_framework *fw)
>   	struct pci_device *head = NULL, *cur = NULL, *device = NULL;
>   	char command[PATH_MAX];
>   	int ret = FWTS_OK;
> +	int status;
>
>   	snprintf(command, sizeof(command), "%s", fw->lspci);
>
> -	if (fwts_pipe_exec(command, &lspci_output) == FWTS_EXEC_ERROR) {
> +	if (fwts_pipe_exec(command, &lspci_output, &status) != FWTS_OK) {
>   		fwts_log_warning(fw, "Could not execute %s", command);
>   		return FWTS_ERROR;
>   	}
> @@ -267,10 +268,11 @@ static int pcie_check_aspm_registers(fwts_framework *fw)
>   	for (cur = head; cur; cur = cur->next) {
>   		int reg_loc = 0, reg_val = 0;
>   		int i;
> +		int status;
>
>   		snprintf(command, sizeof(command), "%s -s %02X:%02X.%02X -xxx",
>   			fw->lspci, cur->bus, cur->dev, cur->func);
> -		if (fwts_pipe_exec(command, &lspci_output) == FWTS_EXEC_ERROR) {
> +		if (fwts_pipe_exec(command, &lspci_output, &status) != FWTS_OK) {
>   			fwts_log_warning(fw, "Could not execute %s", command);
>   			pci_device_list_free(head);
>   			return FWTS_ERROR;
> diff --git a/src/pci/maxreadreq/maxreadreq.c b/src/pci/maxreadreq/maxreadreq.c
> index e5ace46..92af83f 100644
> --- a/src/pci/maxreadreq/maxreadreq.c
> +++ b/src/pci/maxreadreq/maxreadreq.c
> @@ -37,10 +37,12 @@ static fwts_list *lspci_text;
>
>   static int maxreadreq_init(fwts_framework *fw)
>   {
> +	int status;
> +
>   	if (fwts_check_executable(fw, fw->lspci, "lspci"))
>   		return FWTS_ERROR;
>
> -	if (fwts_pipe_exec("lspci -vvv", &lspci_text)) {
> +	if (fwts_pipe_exec("lspci -vvv", &lspci_text, &status) != FWTS_OK) {
>   		fwts_log_error(fw, "Failed to execute lspci -vvv.");
>   		return FWTS_ERROR;
>   	}
>
Acked-by: Alex Hung <alex.hung@canonical.com>

Patch

diff --git a/src/acpi/mcfg/mcfg.c b/src/acpi/mcfg/mcfg.c
index 331c24f..39ae697 100644
--- a/src/acpi/mcfg/mcfg.c
+++ b/src/acpi/mcfg/mcfg.c
@@ -40,6 +40,7 @@  static void compare_config_space(
 {
 	fwts_list *lspci_output;
 	fwts_list_link *item;
+	int status;
 
 	char command[PATH_MAX];
 	char compare_line[50];
@@ -54,7 +55,7 @@  static void compare_config_space(
 	snprintf(command, sizeof(command), "%s -vxxx -s %" PRIu16 ":%" PRIu32,
 		fw->lspci, segment, device);
 
-	if (fwts_pipe_exec(command, &lspci_output) == FWTS_EXEC_ERROR) {
+	if (fwts_pipe_exec(command, &lspci_output, &status) != FWTS_OK) {
 		fwts_log_warning(fw, "Could not execute %s", command);
 		return;
 	}
diff --git a/src/acpi/s3/s3.c b/src/acpi/s3/s3.c
index a1b7848..182db43 100644
--- a/src/acpi/s3/s3.c
+++ b/src/acpi/s3/s3.c
@@ -106,7 +106,7 @@  static int s3_do_suspend_resume(fwts_framework *fw,
 	/* Do S3 here */
 	fwts_progress_message(fw, percent, "(Suspending)");
 	time(&t_start);
-	status = fwts_pipe_exec(command, &output);
+	(void)fwts_pipe_exec(command, &output, &status);
 	time(&t_end);
 	fwts_progress_message(fw, percent, "(Resumed)");
 	fwts_text_list_free(output);
diff --git a/src/acpi/s3power/s3power.c b/src/acpi/s3power/s3power.c
index a8a7c25..0b98df1 100644
--- a/src/acpi/s3power/s3power.c
+++ b/src/acpi/s3power/s3power.c
@@ -191,8 +191,7 @@  static int s3power_test(fwts_framework *fw)
 	/* Do S3 here */
 	fwts_progress_message(fw, 100, "(Suspending)");
 	time(&t_start);
-	status = 0;
-	status = fwts_pipe_exec(PM_SUSPEND, &output);
+	(void)fwts_pipe_exec(PM_SUSPEND, &output, &status);
 	time(&t_end);
 	fwts_progress_message(fw, 100, "(Resumed)");
 	fwts_text_list_free(output);
diff --git a/src/acpi/s4/s4.c b/src/acpi/s4/s4.c
index 728062d..bea7fce 100644
--- a/src/acpi/s4/s4.c
+++ b/src/acpi/s4/s4.c
@@ -137,7 +137,7 @@  static int s4_hibernate(fwts_framework *fw,
 
 	/* Do s4 here */
 	fwts_progress_message(fw, percent, "(Hibernating)");
-	status = fwts_pipe_exec(command, &output);
+	(void)fwts_pipe_exec(command, &output, &status);
 	fwts_progress_message(fw, percent, "(Resumed)");
 	fwts_text_list_free(output);
 	free(command);
diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c
index 6dbc8e2..62a91c7 100644
--- a/src/bios/mtrr/mtrr.c
+++ b/src/bios/mtrr/mtrr.c
@@ -199,11 +199,12 @@  static bool is_prefetchable(fwts_framework *fw, char *device, uint64_t address)
 	char line[4096];
 	fwts_list *lspci_output;
 	fwts_list_link *item;
+	int status;
 
 	memset(line,0,4096);
 
 	snprintf(line, sizeof(line), "%s -v -s %s", fw->lspci, device);
-	fwts_pipe_exec(line, &lspci_output);
+	fwts_pipe_exec(line, &lspci_output, &status);
 	if (lspci_output == NULL)
 		return pref;
 
diff --git a/src/hotkey/hotkey/hotkey.c b/src/hotkey/hotkey/hotkey.c
index 267b69e..59a6829 100644
--- a/src/hotkey/hotkey/hotkey.c
+++ b/src/hotkey/hotkey/hotkey.c
@@ -182,9 +182,10 @@  static char *hotkey_find_keymap(char *device)
 
 	char buffer[1024];
 	char *keymap = NULL;
+	int status;
 
 	snprintf(buffer, sizeof(buffer), "udevadm test /class/%s 2>&1", device);
-	if (fwts_pipe_exec(buffer, &output) != FWTS_OK)
+	if (fwts_pipe_exec(buffer, &output, &status) != FWTS_OK)
 		return NULL;
 
 	snprintf(buffer, sizeof(buffer), "keymap %s", device);
diff --git a/src/lib/include/fwts_pipeio.h b/src/lib/include/fwts_pipeio.h
index f1875da..04f4dea 100644
--- a/src/lib/include/fwts_pipeio.h
+++ b/src/lib/include/fwts_pipeio.h
@@ -31,6 +31,6 @@ 
 int   fwts_pipe_open(const char *command, pid_t *childpid);
 char *fwts_pipe_read(const int fd, ssize_t *length);
 int   fwts_pipe_close(const int fd, const pid_t pid);
-int   fwts_pipe_exec(const char *command, fwts_list **list);
+int   fwts_pipe_exec(const char *command, fwts_list **list, int *status);
 
 #endif
diff --git a/src/lib/src/fwts_efi_module.c b/src/lib/src/fwts_efi_module.c
index 26e5740..c92a091 100644
--- a/src/lib/src/fwts_efi_module.c
+++ b/src/lib/src/fwts_efi_module.c
@@ -57,7 +57,9 @@  int fwts_lib_efi_runtime_load_module(fwts_framework *fw)
 	}
 
 	if (!module_already_loaded) {
-		if (fwts_pipe_exec("modprobe efi_runtime", &output) != FWTS_OK) {
+		int status;
+
+		if (fwts_pipe_exec("modprobe efi_runtime", &output, &status) != FWTS_OK) {
 			fwts_log_error(fw, "Load efi_runtime module error.");
 			return FWTS_ERROR;
 		} else {
@@ -94,7 +96,9 @@  int fwts_lib_efi_runtime_unload_module(fwts_framework *fw)
 		return FWTS_ERROR;
 	}
 	if (module_already_loaded) {
-		if (fwts_pipe_exec("modprobe -r efi_runtime", &output) != FWTS_OK) {
+		int status;
+
+		if (fwts_pipe_exec("modprobe -r efi_runtime", &output, &status) != FWTS_OK) {
 			fwts_log_error(fw, "Unload efi_runtime module error.");
 			return FWTS_ERROR;
 		} else {
diff --git a/src/lib/src/fwts_hwinfo.c b/src/lib/src/fwts_hwinfo.c
index 4365c64..3fcac80 100644
--- a/src/lib/src/fwts_hwinfo.c
+++ b/src/lib/src/fwts_hwinfo.c
@@ -33,15 +33,17 @@  int fwts_hwinfo_get(fwts_framework *fw, fwts_hwinfo *hwinfo)
 {
 	FWTS_UNUSED(fw);
 
-	fwts_pipe_exec("lspci | grep Network", &hwinfo->network);
-	fwts_pipe_exec("lspci | grep Ethernet", &hwinfo->ethernet);
-	fwts_pipe_exec("ifconfig -a | grep -A1 '^\\w'", &hwinfo->ifconfig);
-	fwts_pipe_exec("iwconfig | grep -A1 '^\\w'", &hwinfo->iwconfig);
-	fwts_pipe_exec("hciconfig -a | grep -A2 '^\\w", &hwinfo->hciconfig);
-	fwts_pipe_exec("lspci | grep VGA", &hwinfo->videocard);
-	fwts_pipe_exec("xinput list", &hwinfo->xinput);
-	fwts_pipe_exec("pactl list | grep Sink | grep -v Latency", &hwinfo->pactl);
-	fwts_pipe_exec("pactl list | grep Source | grep -v Latency", &hwinfo->pactl);
+	int status;
+
+	fwts_pipe_exec("lspci | grep Network", &hwinfo->network, &status);
+	fwts_pipe_exec("lspci | grep Ethernet", &hwinfo->ethernet, &status);
+	fwts_pipe_exec("ifconfig -a | grep -A1 '^\\w'", &hwinfo->ifconfig, &status);
+	fwts_pipe_exec("iwconfig | grep -A1 '^\\w'", &hwinfo->iwconfig, &status);
+	fwts_pipe_exec("hciconfig -a | grep -A2 '^\\w", &hwinfo->hciconfig, &status);
+	fwts_pipe_exec("lspci | grep VGA", &hwinfo->videocard, &status);
+	fwts_pipe_exec("xinput list", &hwinfo->xinput, &status);
+	fwts_pipe_exec("pactl list | grep Sink | grep -v Latency", &hwinfo->pactl, &status);
+	fwts_pipe_exec("pactl list | grep Source | grep -v Latency", &hwinfo->pactl, &status);
 
 	return FWTS_OK;
 }
diff --git a/src/lib/src/fwts_pipeio.c b/src/lib/src/fwts_pipeio.c
index 68abee9..5af817b 100644
--- a/src/lib/src/fwts_pipeio.c
+++ b/src/lib/src/fwts_pipeio.c
@@ -133,14 +133,15 @@  int fwts_pipe_close(const int fd, const pid_t pid)
  *  fwts_pipe_exec()
  *	execute a command, return a list containing lines
  *	of the stdout output from the command.
+ *	Return FWTS_OK if the exec worked, FWTS_EXEC_ERROR if
+ *	it failed.  status contains the child exit status.
  */
-int fwts_pipe_exec(const char *command, fwts_list **list)
+int fwts_pipe_exec(const char *command, fwts_list **list, int *status)
 {
 	pid_t 	pid;
 	int	fd;
 	ssize_t	len;
 	char 	*text;
-	int	ret;
 
 	if ((fd = fwts_pipe_open(command, &pid)) < 0)
 		return FWTS_ERROR;
@@ -149,12 +150,11 @@  int fwts_pipe_exec(const char *command, fwts_list **list)
 	*list = fwts_list_from_text(text);
 	free(text);
 
-	ret = fwts_pipe_close(fd, pid);
-
-	if (ret == FWTS_EXEC_ERROR) {
+	*status = fwts_pipe_close(fd, pid);
+	if (*status) {
 		fwts_list_free(*list, free);
 		*list = NULL;
-		return FWTS_ERROR;
+		return FWTS_EXEC_ERROR;
 	}
 	return FWTS_OK;
 }
diff --git a/src/pci/aspm/aspm.c b/src/pci/aspm/aspm.c
index 1a6cc27..a2cda8c 100644
--- a/src/pci/aspm/aspm.c
+++ b/src/pci/aspm/aspm.c
@@ -229,10 +229,11 @@  static int pcie_check_aspm_registers(fwts_framework *fw)
 	struct pci_device *head = NULL, *cur = NULL, *device = NULL;
 	char command[PATH_MAX];
 	int ret = FWTS_OK;
+	int status;
 
 	snprintf(command, sizeof(command), "%s", fw->lspci);
 
-	if (fwts_pipe_exec(command, &lspci_output) == FWTS_EXEC_ERROR) {
+	if (fwts_pipe_exec(command, &lspci_output, &status) != FWTS_OK) {
 		fwts_log_warning(fw, "Could not execute %s", command);
 		return FWTS_ERROR;
 	}
@@ -267,10 +268,11 @@  static int pcie_check_aspm_registers(fwts_framework *fw)
 	for (cur = head; cur; cur = cur->next) {
 		int reg_loc = 0, reg_val = 0;
 		int i;
+		int status;
 
 		snprintf(command, sizeof(command), "%s -s %02X:%02X.%02X -xxx",
 			fw->lspci, cur->bus, cur->dev, cur->func);
-		if (fwts_pipe_exec(command, &lspci_output) == FWTS_EXEC_ERROR) {
+		if (fwts_pipe_exec(command, &lspci_output, &status) != FWTS_OK) {
 			fwts_log_warning(fw, "Could not execute %s", command);
 			pci_device_list_free(head);
 			return FWTS_ERROR;
diff --git a/src/pci/maxreadreq/maxreadreq.c b/src/pci/maxreadreq/maxreadreq.c
index e5ace46..92af83f 100644
--- a/src/pci/maxreadreq/maxreadreq.c
+++ b/src/pci/maxreadreq/maxreadreq.c
@@ -37,10 +37,12 @@  static fwts_list *lspci_text;
 
 static int maxreadreq_init(fwts_framework *fw)
 {
+	int status;
+
 	if (fwts_check_executable(fw, fw->lspci, "lspci"))
 		return FWTS_ERROR;
 
-	if (fwts_pipe_exec("lspci -vvv", &lspci_text)) {
+	if (fwts_pipe_exec("lspci -vvv", &lspci_text, &status) != FWTS_OK) {
 		fwts_log_error(fw, "Failed to execute lspci -vvv.");
 		return FWTS_ERROR;
 	}