From patchwork Fri Dec 7 14:08:49 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Colin Ian King X-Patchwork-Id: 204494 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from chlorine.canonical.com (chlorine.canonical.com [91.189.94.204]) by ozlabs.org (Postfix) with ESMTP id D8B992C0156 for ; Sat, 8 Dec 2012 01:08:54 +1100 (EST) Received: from localhost ([127.0.0.1] helo=chlorine.canonical.com) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1Tgybl-0007kb-4B; Fri, 07 Dec 2012 14:08:53 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1Tgybi-0007kI-CQ for fwts-devel@lists.ubuntu.com; Fri, 07 Dec 2012 14:08:50 +0000 Received: from cpc3-craw6-2-0-cust180.croy.cable.virginmedia.com ([77.100.248.181] helo=localhost) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1Tgybi-0004rI-5e for fwts-devel@lists.ubuntu.com; Fri, 07 Dec 2012 14:08:50 +0000 From: Colin King To: fwts-devel@lists.ubuntu.com Subject: [PATCH] lib + tests: modify fwts_pipe_exec to return the child exit status Date: Fri, 7 Dec 2012 14:08:49 +0000 Message-Id: <1354889329-30567-1-git-send-email-colin.king@canonical.com> X-Mailer: git-send-email 1.8.0 X-BeenThere: fwts-devel@lists.ubuntu.com X-Mailman-Version: 2.1.13 Precedence: list List-Id: Firmware Test Suite Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: fwts-devel-bounces@lists.ubuntu.com Errors-To: fwts-devel-bounces@lists.ubuntu.com From: Colin Ian King 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 Acked-by: Keng-Yu Lin Acked-by: Alex Hung --- 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; }