Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added logging for calls to system(3) #157

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Added logging for calls to system(3) #157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6304fc7
Added logging for calls to system(3)
May 18, 2022
2cf34a4
Fix dying without a message on failed verbose_system calls, improve i…
Jun 2, 2022
fb3bc93
comply with formatting
Aug 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions gtests/net/packetdrill/net_utils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,7 @@
#include <unistd.h>

#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,
Expand Down
35 changes: 12 additions & 23 deletions gtests/net/packetdrill/netdev.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
#include "packet.h"
#include "packet_parser.h"
#include "packet_socket.h"
#include "system.h"
#include "tcp.h"
#include "tun.h"

Expand Down Expand Up @@ -92,23 +93,14 @@ 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;
}
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__) */
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -275,16 +267,16 @@ 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
* will be copied to the link_speed field. This field is
* 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);
}
Expand All @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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
Expand Down
40 changes: 30 additions & 10 deletions gtests/net/packetdrill/system.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,41 @@
#include <sys/types.h>
#include <sys/wait.h>

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;
}
alfunke marked this conversation as resolved.
Show resolved Hide resolved
}

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;
Expand Down
6 changes: 6 additions & 0 deletions gtests/net/packetdrill/system.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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__ */
3 changes: 2 additions & 1 deletion gtests/net/packetdrill/wire_client_netdev.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

#include "logging.h"
#include "net_utils.h"
#include "system.h"

struct wire_client_netdev {
struct netdev netdev; /* "inherit" from netdev */
Expand Down Expand Up @@ -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);
}
Expand Down
5 changes: 3 additions & 2 deletions gtests/net/packetdrill/wire_server_netdev.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand Down