diff --git a/gtests/net/packetdrill/net_utils.c b/gtests/net/packetdrill/net_utils.c index 401a9877..6978bea9 100644 --- a/gtests/net/packetdrill/net_utils.c +++ b/gtests/net/packetdrill/net_utils.c @@ -29,17 +29,7 @@ #include #include "logging.h" - -static void verbose_system(const char *command) -{ - int result; - - DEBUGP("running: '%s'\n", command); - result = system(command); - DEBUGP("result: %d\n", result); - if (result != 0) - DEBUGP("error executing command '%s'\n", command); -} +#include "system.h" /* Configure a local IPv4 address and netmask for the device */ static void net_add_ipv4_address(const char *dev_name, diff --git a/gtests/net/packetdrill/netdev.c b/gtests/net/packetdrill/netdev.c index fb0edd71..1fabbbab 100644 --- a/gtests/net/packetdrill/netdev.c +++ b/gtests/net/packetdrill/netdev.c @@ -59,6 +59,7 @@ #include "packet.h" #include "packet_parser.h" #include "packet_socket.h" +#include "system.h" #include "tcp.h" #include "tun.h" @@ -92,9 +93,6 @@ static void cleanup_old_device(struct config *config, { #if defined(__NetBSD__) char *cleanup_command = NULL; -#ifdef DEBUG - int result; -#endif if ((config->tun_device == NULL) || config->persistent_tun_device) { return; @@ -102,13 +100,7 @@ static void cleanup_old_device(struct config *config, asprintf(&cleanup_command, "/sbin/ifconfig %s down delete > /dev/null 2>&1", config->tun_device); - DEBUGP("running: '%s'\n", cleanup_command); -#ifdef DEBUG - result = system(cleanup_command); -#else - system(cleanup_command); -#endif - DEBUGP("result: %d\n", result); + verbose_system(cleanup_command); free(cleanup_command); #endif /* defined(__NetBSD__) */ } @@ -166,7 +158,7 @@ static void create_device(struct config *config, struct local_netdev *netdev) DEBUGP("utun index: '%d'\n", netdev->index); if (config->mtu != TUN_DRIVER_DEFAULT_MTU) { asprintf(&command, "ifconfig %s mtu %d", netdev->name, config->mtu); - if (system(command) < 0) + if (verbose_system(command) != STATUS_OK) die("Error executing %s\n", command); free(command); } @@ -199,7 +191,7 @@ static void create_device(struct config *config, struct local_netdev *netdev) tun_fd = open(tun_path, O_RDWR); #if defined(__FreeBSD__) if ((tun_fd < 0) && (errno == ENOENT)) { - if (system("kldload -q if_tun") < 0) { + if (verbose_system("kldload -q if_tun") != STATUS_OK) { die_perror("kldload -q if_tun"); } tun_fd = open(tun_path, O_RDWR); @@ -275,8 +267,8 @@ static void create_device(struct config *config, struct local_netdev *netdev) char *command; asprintf(&command, "ethtool -s %s speed %u autoneg off", netdev->name, config->speed); - if (system(command) < 0) - die("Error executing %s\n", command); + if (verbose_system(command) != STATUS_OK) + die("Error executing %s\n", command); free(command); /* Need to bring interface down and up so the interface speed @@ -284,7 +276,7 @@ static void create_device(struct config *config, struct local_netdev *netdev) * used by TCP's cwnd bound. */ asprintf(&command, "ifconfig %s down; sleep 1; ifconfig %s up; " "sleep 1", netdev->name, netdev->name); - if (system(command) < 0) + if (verbose_system(command) != STATUS_OK) die("Error executing %s\n", command); free(command); } @@ -293,8 +285,8 @@ static void create_device(struct config *config, struct local_netdev *netdev) char *command; asprintf(&command, "ifconfig %s mtu %d", netdev->name, config->mtu); - if (system(command) < 0) - die("Error executing %s\n", command); + if (verbose_system(command) != STATUS_OK) + die("Error executing route command '%s'\n", command); free(command); } #endif @@ -377,11 +369,8 @@ static void route_traffic_to_device(struct config *config, assert(!"bad wire protocol"); } #endif /* defined(linux) */ - int result = system(route_command); - if ((result == -1) || (WEXITSTATUS(result) != 0)) { - die("error executing route command '%s'\n", - route_command); - } + if (verbose_system(command) != STATUS_OK) + die("Error executing %s\n", command); free(route_command); } @@ -433,7 +422,7 @@ static void local_netdev_free(struct netdev *a_netdev) asprintf(&cleanup_command, "/sbin/ifconfig %s destroy > /dev/null 2>&1", netdev->name); - system(cleanup_command); + verbose_system(cleanup_command); free(cleanup_command); } #endif diff --git a/gtests/net/packetdrill/system.c b/gtests/net/packetdrill/system.c index 1b17abe5..fe219468 100644 --- a/gtests/net/packetdrill/system.c +++ b/gtests/net/packetdrill/system.c @@ -31,21 +31,41 @@ #include #include -int safe_system(const char *command, char **error) +#include "logging.h" + +static void checked_system(const char *command, char **error) { - int status = system(command); + int status; + status = system(command); if (status == -1) { asprintf(error, "%s", strerror(errno)); - return STATUS_ERR; - } - if (WIFSIGNALED(status) && - (WTERMSIG(status) == SIGINT || WTERMSIG(status) == SIGQUIT)) { + } else if (WIFSIGNALED(status) && + (WTERMSIG(status) == SIGINT || WTERMSIG(status) == SIGQUIT)) { asprintf(error, "got signal %d (%s)", - WTERMSIG(status), strsignal(WTERMSIG(status))); - return STATUS_ERR; - } - if (WEXITSTATUS(status) != 0) { + WTERMSIG(status), strsignal(WTERMSIG(status))); + } else if (WEXITSTATUS(status) != 0) { asprintf(error, "non-zero status %d", WEXITSTATUS(status)); + } else { + *error = NULL; + } +} + +int safe_system(const char *command, char **error) +{ + checked_system(command, error); + if (*error != NULL) + return STATUS_ERR; + return STATUS_OK; +} + +int verbose_system(const char *command) +{ + char *error = NULL; + + DEBUGP("running: '%s'\n", command); + checked_system(command, &error); + if (*error != NULL) { + DEBUGP("error: %s executing command '%s'\n", error, command); return STATUS_ERR; } return STATUS_OK; diff --git a/gtests/net/packetdrill/system.h b/gtests/net/packetdrill/system.h index 1f7564d0..727f1d36 100644 --- a/gtests/net/packetdrill/system.h +++ b/gtests/net/packetdrill/system.h @@ -32,4 +32,10 @@ */ extern int safe_system(const char *command, char **error); +/* Execute the given command with system(3). + * If debug logging is active, logs the command and result or error. + * On success, returns STATUS_OK. On error returns STATUS_ERR. + */ +extern int verbose_system(const char *command); + #endif /* __SYSTEM_H__ */ diff --git a/gtests/net/packetdrill/wire_client_netdev.c b/gtests/net/packetdrill/wire_client_netdev.c index 6c580990..05455524 100644 --- a/gtests/net/packetdrill/wire_client_netdev.c +++ b/gtests/net/packetdrill/wire_client_netdev.c @@ -31,6 +31,7 @@ #include "logging.h" #include "net_utils.h" +#include "system.h" struct wire_client_netdev { struct netdev netdev; /* "inherit" from netdev */ @@ -99,7 +100,7 @@ static void route_traffic_to_wire_server(struct config *config, * since they can happen if there is no previously existing * route. */ - system(route_command); + verbose_system(route_command); free(route_command); } diff --git a/gtests/net/packetdrill/wire_server_netdev.c b/gtests/net/packetdrill/wire_server_netdev.c index 5c37898d..63b30718 100644 --- a/gtests/net/packetdrill/wire_server_netdev.c +++ b/gtests/net/packetdrill/wire_server_netdev.c @@ -35,6 +35,7 @@ #include "packet.h" #include "packet_socket.h" #include "packet_parser.h" +#include "system.h" struct wire_server_netdev { struct netdev netdev; /* "inherit" from netdev */ @@ -76,7 +77,7 @@ void wire_server_netdev_init(const char *netdev_name) netdev_name); /* For now, intentionally ignoring errors. TODO: clean up. */ - system(command); + verbose_system(command); free(command); /* Block outgoing IPv6 "destination unreachable" messages, to @@ -89,7 +90,7 @@ void wire_server_netdev_init(const char *netdev_name) "ip6tables -F OUTPUT; " "ip6tables -A OUTPUT -p icmpv6 --icmpv6-type 1 -j DROP"); /* For now, intentionally ignoring. TODO: clean up. */ - system(command); + verbose_system(command); free(command); #endif }