* [PATCH 0/3] conf, log: Fix regression in usage() printing
@ 2024-05-29 9:04 David Gibson
2024-05-29 9:04 ` [PATCH 1/3] conf: Remove unhelpful usage() wrapper David Gibson
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: David Gibson @ 2024-05-29 9:04 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: erik.sjolund, David Gibson
Erik Sjölund pointed out a regression where we're no long printing the
--help message to stdout, but to stderr instead.
I'm not actually sure what caused the regression, but it was made
possible by the fact that the original fix was pretty strange to begin
with. It introduced some hacks to force the logging subsystem to go
to stdout, which have presumably failed in some subtle way. But,
there's really no reason to run usage() through the logging subsystem
in the first place - it just adds complexity.
Link: https://bugs.passt.top/show_bug.cgi?id=90
Link: https://bugs.passt.top/show_bug.cgi?id=52
David Gibson (3):
conf: Remove unhelpful usage() wrapper
conf: Don't print usage via the logging subsystem
log: Remove log_to_stdout option
conf.c | 329 ++++++++++++++++++++++++++++-----------------------------
log.c | 8 +-
log.h | 1 -
3 files changed, 163 insertions(+), 175 deletions(-)
--
2.45.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] conf: Remove unhelpful usage() wrapper
2024-05-29 9:04 [PATCH 0/3] conf, log: Fix regression in usage() printing David Gibson
@ 2024-05-29 9:04 ` David Gibson
2024-05-29 9:04 ` [PATCH 2/3] conf: Don't print usage via the logging subsystem David Gibson
2024-05-29 9:04 ` [PATCH 3/3] log: Remove log_to_stdout option David Gibson
2 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2024-05-29 9:04 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: erik.sjolund, David Gibson
usage() does nothing but call print_usage() with EXIT_FAILURE as a
parameter. It's no more complex to just give that parameter at the single
call site. Eliminate it and rename print_usage() to just usage().
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/conf.c b/conf.c
index 50383a39..f2a92574 100644
--- a/conf.c
+++ b/conf.c
@@ -704,11 +704,11 @@ static unsigned int conf_ip6(unsigned int ifi,
}
/**
- * print_usage() - Print usage, exit with given status code
+ * usage() - Print usage, exit with given status code
* @name: Executable name
* @status: Status code for exit()
*/
-static void print_usage(const char *name, int status)
+static void usage(const char *name, int status)
{
if (strstr(name, "pasta")) {
info("Usage: %s [OPTION]... [COMMAND] [ARGS]...", name);
@@ -897,15 +897,6 @@ pasta_opts:
exit(status);
}
-/**
- * usage() - Print usage and exit with failure
- * @name: Executable name
- */
-static void usage(const char *name)
-{
- print_usage(name, EXIT_FAILURE);
-}
-
/**
* conf_print() - Print fundamental configuration parameters
* @c: Execution context
@@ -1647,11 +1638,11 @@ void conf(struct ctx *c, int argc, char **argv)
break;
case 'h':
log_to_stdout = 1;
- print_usage(argv[0], EXIT_SUCCESS);
+ usage(argv[0], EXIT_SUCCESS);
break;
case '?':
default:
- usage(argv[0]);
+ usage(argv[0], EXIT_FAILURE);
break;
}
} while (name != -1);
--
@@ -704,11 +704,11 @@ static unsigned int conf_ip6(unsigned int ifi,
}
/**
- * print_usage() - Print usage, exit with given status code
+ * usage() - Print usage, exit with given status code
* @name: Executable name
* @status: Status code for exit()
*/
-static void print_usage(const char *name, int status)
+static void usage(const char *name, int status)
{
if (strstr(name, "pasta")) {
info("Usage: %s [OPTION]... [COMMAND] [ARGS]...", name);
@@ -897,15 +897,6 @@ pasta_opts:
exit(status);
}
-/**
- * usage() - Print usage and exit with failure
- * @name: Executable name
- */
-static void usage(const char *name)
-{
- print_usage(name, EXIT_FAILURE);
-}
-
/**
* conf_print() - Print fundamental configuration parameters
* @c: Execution context
@@ -1647,11 +1638,11 @@ void conf(struct ctx *c, int argc, char **argv)
break;
case 'h':
log_to_stdout = 1;
- print_usage(argv[0], EXIT_SUCCESS);
+ usage(argv[0], EXIT_SUCCESS);
break;
case '?':
default:
- usage(argv[0]);
+ usage(argv[0], EXIT_FAILURE);
break;
}
} while (name != -1);
--
2.45.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] conf: Don't print usage via the logging subsystem
2024-05-29 9:04 [PATCH 0/3] conf, log: Fix regression in usage() printing David Gibson
2024-05-29 9:04 ` [PATCH 1/3] conf: Remove unhelpful usage() wrapper David Gibson
@ 2024-05-29 9:04 ` David Gibson
2024-06-04 22:40 ` Stefano Brivio
2024-05-29 9:04 ` [PATCH 3/3] log: Remove log_to_stdout option David Gibson
2 siblings, 1 reply; 6+ messages in thread
From: David Gibson @ 2024-05-29 9:04 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: erik.sjolund, David Gibson
The message from usage() when given invalid options, or the -h / --help
option is currently printed by many calls to the info() function, also
used for runtime logging of informational messages.
That isn't useful: the usage message should always go to the terminal
(stdout or stderr), never syslog or a logfile. It should never be
filtered by priority. Really the only thing using the common logging
functions does is give more opportunities for something to go wrong.
Replace all the info() calls with direct fprintf() calls. This does mean
manually adding "\n" to each message. A little messy, but worth it for the
simplicity in other dimensions.
Link: https://bugs.passt.top/show_bug.cgi?id=90
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
conf.c | 318 ++++++++++++++++++++++++++++-----------------------------
1 file changed, 159 insertions(+), 159 deletions(-)
diff --git a/conf.c b/conf.c
index f2a92574..31f5b197 100644
--- a/conf.c
+++ b/conf.c
@@ -706,193 +706,194 @@ static unsigned int conf_ip6(unsigned int ifi,
/**
* usage() - Print usage, exit with given status code
* @name: Executable name
+ * @f: Stream to print usage info to
* @status: Status code for exit()
*/
-static void usage(const char *name, int status)
+static void usage(const char *name, FILE *f, int status)
{
if (strstr(name, "pasta")) {
- info("Usage: %s [OPTION]... [COMMAND] [ARGS]...", name);
- info(" %s [OPTION]... PID", name);
- info(" %s [OPTION]... --netns [PATH|NAME]", name);
- info("");
- info("Without PID or --netns, run the given command or a");
- info("default shell in a new network and user namespace, and");
- info("connect it via pasta.");
+ fprintf(f, "Usage: %s [OPTION]... [COMMAND] [ARGS]...\n", name);
+ fprintf(f, " %s [OPTION]... PID\n", name);
+ fprintf(f, " %s [OPTION]... --netns [PATH|NAME]\n", name);
+ fprintf(f, "\n");
+ fprintf(f, "Without PID or --netns, run the given command or a\n");
+ fprintf(f, "default shell in a new network and user namespace, and\n");
+ fprintf(f, "connect it via pasta.\n");
} else {
- info("Usage: %s [OPTION]...", name);
+ fprintf(f, "Usage: %s [OPTION]...\n", name);
}
- info("");
-
-
- info( " -d, --debug Be verbose");
- info( " --trace Be extra verbose, implies --debug");
- info( " -q, --quiet Don't print informational messages");
- info( " -f, --foreground Don't run in background");
- info( " default: run in background if started from a TTY");
- info( " -e, --stderr Log to stderr too");
- info( " default: log to system logger only if started from a TTY");
- info( " -l, --log-file PATH Log (only) to given file");
- info( " --log-size BYTES Maximum size of log file");
- info( " default: 1 MiB");
- info( " --runas UID|UID:GID Run as given UID, GID, which can be");
- info( " numeric, or login and group names");
- info( " default: drop to user \"nobody\"");
- info( " -h, --help Display this help message and exit");
- info( " --version Show version and exit");
+ fprintf(f, "\n");
+
+
+ fprintf(f, " -d, --debug Be verbose\n");
+ fprintf(f, " --trace Be extra verbose, implies --debug\n");
+ fprintf(f, " -q, --quiet Don't print informational messages\n");
+ fprintf(f, " -f, --foreground Don't run in background\n");
+ fprintf(f, " default: run in background if started from a TTY\n");
+ fprintf(f, " -e, --stderr Log to stderr too\n");
+ fprintf(f, " default: log to system logger only if started from a TTY\n");
+ fprintf(f, " -l, --log-file PATH Log (only) to given file\n");
+ fprintf(f, " --log-size BYTES Maximum size of log file\n");
+ fprintf(f, " default: 1 MiB\n");
+ fprintf(f, " --runas UID|UID:GID Run as given UID, GID, which can be\n");
+ fprintf(f, " numeric, or login and group names\n");
+ fprintf(f, " default: drop to user \"nobody\"\n");
+ fprintf(f, " -h, --help Display this help message and exit\n");
+ fprintf(f, " --version Show version and exit\n");
if (strstr(name, "pasta")) {
- info( " -I, --ns-ifname NAME namespace interface name");
- info( " default: same interface name as external one");
+ fprintf(f, " -I, --ns-ifname NAME namespace interface name\n");
+ fprintf(f, " default: same interface name as external one\n");
} else {
- info( " -s, --socket PATH UNIX domain socket path");
- info( " default: probe free path starting from "
- UNIX_SOCK_PATH, 1);
+ fprintf(f, " -s, --socket PATH UNIX domain socket path\n");
+ fprintf(f, " default: probe free path starting from "
+ UNIX_SOCK_PATH "\n", 1);
}
- info( " -F, --fd FD Use FD as pre-opened connected socket");
- info( " -p, --pcap FILE Log tap-facing traffic to pcap file");
- info( " -P, --pid FILE Write own PID to the given file");
- info( " -m, --mtu MTU Assign MTU via DHCP/NDP");
- info( " a zero value disables assignment");
- info( " default: 65520: maximum 802.3 MTU minus 802.3 header");
- info( " length, rounded to 32 bits (IPv4 words)");
- info( " -a, --address ADDR Assign IPv4 or IPv6 address ADDR");
- info( " can be specified zero to two times (for IPv4 and IPv6)");
- info( " default: use addresses from interface with default route");
- info( " -n, --netmask MASK Assign IPv4 MASK, dot-decimal or bits");
- info( " default: netmask from matching address on the host");
- info( " -M, --mac-addr ADDR Use source MAC address ADDR");
- info( " default: MAC address from interface with default route");
- info( " -g, --gateway ADDR Pass IPv4 or IPv6 address as gateway");
- info( " default: gateway from interface with default route");
- info( " -i, --interface NAME Interface for addresses and routes");
- info( " default: from --outbound-if4 and --outbound-if6, if any");
- info( " otherwise interface with first default route");
- info( " -o, --outbound ADDR Bind to address as outbound source");
- info( " can be specified zero to two times (for IPv4 and IPv6)");
- info( " default: use source address from routing tables");
- info( " --outbound-if4 NAME Bind to outbound interface for IPv4");
- info( " default: use interface from default route");
- info( " --outbound-if6 NAME Bind to outbound interface for IPv6");
- info( " default: use interface from default route");
- info( " -D, --dns ADDR Use IPv4 or IPv6 address as DNS");
- info( " can be specified multiple times");
- info( " a single, empty option disables DNS information");
+ fprintf(f, " -F, --fd FD Use FD as pre-opened connected socket\n");
+ fprintf(f, " -p, --pcap FILE Log tap-facing traffic to pcap file\n");
+ fprintf(f, " -P, --pid FILE Write own PID to the given file\n");
+ fprintf(f, " -m, --mtu MTU Assign MTU via DHCP/NDP\n");
+ fprintf(f, " a zero value disables assignment\n");
+ fprintf(f, " default: 65520: maximum 802.3 MTU minus 802.3 header\n");
+ fprintf(f, " length, rounded to 32 bits (IPv4 words)\n");
+ fprintf(f, " -a, --address ADDR Assign IPv4 or IPv6 address ADDR\n");
+ fprintf(f, " can be specified zero to two times (for IPv4 and IPv6)\n");
+ fprintf(f, " default: use addresses from interface with default route\n");
+ fprintf(f, " -n, --netmask MASK Assign IPv4 MASK, dot-decimal or bits\n");
+ fprintf(f, " default: netmask from matching address on the host\n");
+ fprintf(f, " -M, --mac-addr ADDR Use source MAC address ADDR\n");
+ fprintf(f, " default: MAC address from interface with default route\n");
+ fprintf(f, " -g, --gateway ADDR Pass IPv4 or IPv6 address as gateway\n");
+ fprintf(f, " default: gateway from interface with default route\n");
+ fprintf(f, " -i, --interface NAME Interface for addresses and routes\n");
+ fprintf(f, " default: from --outbound-if4 and --outbound-if6, if any\n");
+ fprintf(f, " otherwise interface with first default route\n");
+ fprintf(f, " -o, --outbound ADDR Bind to address as outbound source\n");
+ fprintf(f, " can be specified zero to two times (for IPv4 and IPv6)\n");
+ fprintf(f, " default: use source address from routing tables\n");
+ fprintf(f, " --outbound-if4 NAME Bind to outbound interface for IPv4\n");
+ fprintf(f, " default: use interface from default route\n");
+ fprintf(f, " --outbound-if6 NAME Bind to outbound interface for IPv6\n");
+ fprintf(f, " default: use interface from default route\n");
+ fprintf(f, " -D, --dns ADDR Use IPv4 or IPv6 address as DNS\n");
+ fprintf(f, " can be specified multiple times\n");
+ fprintf(f, " a single, empty option disables DNS information\n");
if (strstr(name, "pasta"))
- info( " default: don't use any addresses");
+ fprintf(f, " default: don't use any addresses\n");
else
- info( " default: use addresses from /etc/resolv.conf");
+ fprintf(f, " default: use addresses from /etc/resolv.conf\n");
- info( " -S, --search LIST Space-separated list, search domains");
- info( " a single, empty option disables the DNS search list");
+ fprintf(f, " -S, --search LIST Space-separated list, search domains\n");
+ fprintf(f, " a single, empty option disables the DNS search list\n");
if (strstr(name, "pasta"))
- info( " default: don't use any search list");
+ fprintf(f, " default: don't use any search list\n");
else
- info( " default: use search list from /etc/resolv.conf");
+ fprintf(f, " default: use search list from /etc/resolv.conf\n");
if (strstr(name, "pasta"))
- info(" --dhcp-dns \tPass DNS list via DHCP/DHCPv6/NDP");
+ fprintf(f, " --dhcp-dns \tPass DNS list via DHCP/DHCPv6/NDP\n");
else
- info(" --no-dhcp-dns No DNS list in DHCP/DHCPv6/NDP");
+ fprintf(f, " --no-dhcp-dns No DNS list in DHCP/DHCPv6/NDP\n");
if (strstr(name, "pasta"))
- info(" --dhcp-search Pass list via DHCP/DHCPv6/NDP");
+ fprintf(f, " --dhcp-search Pass list via DHCP/DHCPv6/NDP\n");
else
- info(" --no-dhcp-search No list in DHCP/DHCPv6/NDP");
-
- info( " --dns-forward ADDR Forward DNS queries sent to ADDR");
- info( " can be specified zero to two times (for IPv4 and IPv6)");
- info( " default: don't forward DNS queries");
-
- info( " --no-tcp Disable TCP protocol handler");
- info( " --no-udp Disable UDP protocol handler");
- info( " --no-icmp Disable ICMP/ICMPv6 protocol handler");
- info( " --no-dhcp Disable DHCP server");
- info( " --no-ndp Disable NDP responses");
- info( " --no-dhcpv6 Disable DHCPv6 server");
- info( " --no-ra Disable router advertisements");
- info( " --no-map-gw Don't map gateway address to host");
- info( " -4, --ipv4-only Enable IPv4 operation only");
- info( " -6, --ipv6-only Enable IPv6 operation only");
+ fprintf(f, " --no-dhcp-search No list in DHCP/DHCPv6/NDP\n");
+
+ fprintf(f, " --dns-forward ADDR Forward DNS queries sent to ADDR\n");
+ fprintf(f, " can be specified zero to two times (for IPv4 and IPv6)\n");
+ fprintf(f, " default: don't forward DNS queries\n");
+
+ fprintf(f, " --no-tcp Disable TCP protocol handler\n");
+ fprintf(f, " --no-udp Disable UDP protocol handler\n");
+ fprintf(f, " --no-icmp Disable ICMP/ICMPv6 protocol handler\n");
+ fprintf(f, " --no-dhcp Disable DHCP server\n");
+ fprintf(f, " --no-ndp Disable NDP responses\n");
+ fprintf(f, " --no-dhcpv6 Disable DHCPv6 server\n");
+ fprintf(f, " --no-ra Disable router advertisements\n");
+ fprintf(f, " --no-map-gw Don't map gateway address to host\n");
+ fprintf(f, " -4, --ipv4-only Enable IPv4 operation only\n");
+ fprintf(f, " -6, --ipv6-only Enable IPv6 operation only\n");
if (strstr(name, "pasta"))
goto pasta_opts;
- info( " -1, --one-off Quit after handling one single client");
- info( " -t, --tcp-ports SPEC TCP port forwarding to guest");
- info( " can be specified multiple times");
- info( " SPEC can be:");
- info( " 'none': don't forward any ports");
- info( " 'all': forward all unbound, non-ephemeral ports");
- info( " a comma-separated list, optionally ranged with '-'");
- info( " and optional target ports after ':', with optional");
- info( " address specification suffixed by '/' and optional");
- info( " interface prefixed by '%%'. Ranges can be reduced by");
- info( " excluding ports or ranges prefixed by '~'");
- info( " Examples:");
- info( " -t 22 Forward local port 22 to 22 on guest");
- info( " -t 22:23 Forward local port 22 to 23 on guest");
- info( " -t 22,25 Forward ports 22, 25 to ports 22, 25");
- info( " -t 22-80 Forward ports 22 to 80");
- info( " -t 22-80:32-90 Forward ports 22 to 80 to");
- info( " corresponding port numbers plus 10");
- info( " -t 192.0.2.1/5 Bind port 5 of 192.0.2.1 to guest");
- info( " -t 5-25,~10-20 Forward ports 5 to 9, and 21 to 25");
- info( " -t ~25 Forward all ports except for 25");
- info( " default: none");
- info( " -u, --udp-ports SPEC UDP port forwarding to guest");
- info( " SPEC is as described for TCP above");
- info( " default: none");
+ fprintf(f, " -1, --one-off Quit after handling one single client\n");
+ fprintf(f, " -t, --tcp-ports SPEC TCP port forwarding to guest\n");
+ fprintf(f, " can be specified multiple times\n");
+ fprintf(f, " SPEC can be:\n");
+ fprintf(f, " 'none': don't forward any ports\n");
+ fprintf(f, " 'all': forward all unbound, non-ephemeral ports\n");
+ fprintf(f, " a comma-separated list, optionally ranged with '-'\n");
+ fprintf(f, " and optional target ports after ':', with optional\n");
+ fprintf(f, " address specification suffixed by '/' and optional\n");
+ fprintf(f, " interface prefixed by '%%'. Ranges can be reduced by\n");
+ fprintf(f, " excluding ports or ranges prefixed by '~'\n");
+ fprintf(f, " Examples:\n");
+ fprintf(f, " -t 22 Forward local port 22 to 22 on guest\n");
+ fprintf(f, " -t 22:23 Forward local port 22 to 23 on guest\n");
+ fprintf(f, " -t 22,25 Forward ports 22, 25 to ports 22, 25\n");
+ fprintf(f, " -t 22-80 Forward ports 22 to 80\n");
+ fprintf(f, " -t 22-80:32-90 Forward ports 22 to 80 to\n");
+ fprintf(f, " corresponding port numbers plus 10\n");
+ fprintf(f, " -t 192.0.2.1/5 Bind port 5 of 192.0.2.1 to guest\n");
+ fprintf(f, " -t 5-25,~10-20 Forward ports 5 to 9, and 21 to 25\n");
+ fprintf(f, " -t ~25 Forward all ports except for 25\n");
+ fprintf(f, " default: none\n");
+ fprintf(f, " -u, --udp-ports SPEC UDP port forwarding to guest\n");
+ fprintf(f, " SPEC is as described for TCP above\n");
+ fprintf(f, " default: none\n");
exit(status);
pasta_opts:
- info( " -t, --tcp-ports SPEC TCP port forwarding to namespace");
- info( " can be specified multiple times");
- info( " SPEC can be:");
- info( " 'none': don't forward any ports");
- info( " 'auto': forward all ports currently bound in namespace");
- info( " a comma-separated list, optionally ranged with '-'");
- info( " and optional target ports after ':', with optional");
- info( " address specification suffixed by '/' and optional");
- info( " interface prefixed by '%%'. Examples:");
- info( " -t 22 Forward local port 22 to port 22 in netns");
- info( " -t 22:23 Forward local port 22 to port 23");
- info( " -t 22,25 Forward ports 22, 25 to ports 22, 25");
- info( " -t 22-80 Forward ports 22 to 80");
- info( " -t 22-80:32-90 Forward ports 22 to 80 to");
- info( " corresponding port numbers plus 10");
- info( " -t 192.0.2.1/5 Bind port 5 of 192.0.2.1 to namespace");
- info( " -t 5-25,~10-20 Forward ports 5 to 9, and 21 to 25");
- info( " -t ~25 Forward all bound ports except for 25");
- info( " default: auto");
- info( " IPv6 bound ports are also forwarded for IPv4");
- info( " -u, --udp-ports SPEC UDP port forwarding to namespace");
- info( " SPEC is as described for TCP above");
- info( " default: auto");
- info( " IPv6 bound ports are also forwarded for IPv4");
- info( " unless specified, with '-t auto', UDP ports with numbers");
- info( " corresponding to forwarded TCP port numbers are");
- info( " forwarded too");
- info( " -T, --tcp-ns SPEC TCP port forwarding to init namespace");
- info( " SPEC is as described above");
- info( " default: auto");
- info( " -U, --udp-ns SPEC UDP port forwarding to init namespace");
- info( " SPEC is as described above");
- info( " default: auto");
- info( " --userns NSPATH Target user namespace to join");
- info( " --netns PATH|NAME Target network namespace to join");
- info( " --netns-only Don't join existing user namespace");
- info( " implied if PATH or NAME are given without --userns");
- info( " --no-netns-quit Don't quit if filesystem-bound target");
- info( " network namespace is deleted");
- info( " --config-net Configure tap interface in namespace");
- info( " --no-copy-routes DEPRECATED:");
- info( " Don't copy all routes to namespace");
- info( " --no-copy-addrs DEPRECATED:");
- info( " Don't copy all addresses to namespace");
- info( " --ns-mac-addr ADDR Set MAC address on tap interface");
+ fprintf(f, " -t, --tcp-ports SPEC TCP port forwarding to namespace\n");
+ fprintf(f, " can be specified multiple times\n");
+ fprintf(f, " SPEC can be:\n");
+ fprintf(f, " 'none': don't forward any ports\n");
+ fprintf(f, " 'auto': forward all ports currently bound in namespace\n");
+ fprintf(f, " a comma-separated list, optionally ranged with '-'\n");
+ fprintf(f, " and optional target ports after ':', with optional\n");
+ fprintf(f, " address specification suffixed by '/' and optional\n");
+ fprintf(f, " interface prefixed by '%%'. Examples:\n");
+ fprintf(f, " -t 22 Forward local port 22 to port 22 in netns\n");
+ fprintf(f, " -t 22:23 Forward local port 22 to port 23\n");
+ fprintf(f, " -t 22,25 Forward ports 22, 25 to ports 22, 25\n");
+ fprintf(f, " -t 22-80 Forward ports 22 to 80\n");
+ fprintf(f, " -t 22-80:32-90 Forward ports 22 to 80 to\n");
+ fprintf(f, " corresponding port numbers plus 10\n");
+ fprintf(f, " -t 192.0.2.1/5 Bind port 5 of 192.0.2.1 to namespace\n");
+ fprintf(f, " -t 5-25,~10-20 Forward ports 5 to 9, and 21 to 25\n");
+ fprintf(f, " -t ~25 Forward all bound ports except for 25\n");
+ fprintf(f, " default: auto\n");
+ fprintf(f, " IPv6 bound ports are also forwarded for IPv4\n");
+ fprintf(f, " -u, --udp-ports SPEC UDP port forwarding to namespace\n");
+ fprintf(f, " SPEC is as described for TCP above\n");
+ fprintf(f, " default: auto\n");
+ fprintf(f, " IPv6 bound ports are also forwarded for IPv4\n");
+ fprintf(f, " unless specified, with '-t auto', UDP ports with numbers\n");
+ fprintf(f, " corresponding to forwarded TCP port numbers are\n");
+ fprintf(f, " forwarded too\n");
+ fprintf(f, " -T, --tcp-ns SPEC TCP port forwarding to init namespace\n");
+ fprintf(f, " SPEC is as described above\n");
+ fprintf(f, " default: auto\n");
+ fprintf(f, " -U, --udp-ns SPEC UDP port forwarding to init namespace\n");
+ fprintf(f, " SPEC is as described above\n");
+ fprintf(f, " default: auto\n");
+ fprintf(f, " --userns NSPATH Target user namespace to join\n");
+ fprintf(f, " --netns PATH|NAME Target network namespace to join\n");
+ fprintf(f, " --netns-only Don't join existing user namespace\n");
+ fprintf(f, " implied if PATH or NAME are given without --userns\n");
+ fprintf(f, " --no-netns-quit Don't quit if filesystem-bound target\n");
+ fprintf(f, " network namespace is deleted\n");
+ fprintf(f, " --config-net Configure tap interface in namespace\n");
+ fprintf(f, " --no-copy-routes DEPRECATED:\n");
+ fprintf(f, " Don't copy all routes to namespace\n");
+ fprintf(f, " --no-copy-addrs DEPRECATED:\n");
+ fprintf(f, " Don't copy all addresses to namespace\n");
+ fprintf(f, " --ns-mac-addr ADDR Set MAC address on tap interface\n");
exit(status);
}
@@ -1637,12 +1638,11 @@ void conf(struct ctx *c, int argc, char **argv)
/* Handle these later, once addresses are configured */
break;
case 'h':
- log_to_stdout = 1;
- usage(argv[0], EXIT_SUCCESS);
+ usage(argv[0], stdout, EXIT_SUCCESS);
break;
case '?':
default:
- usage(argv[0], EXIT_FAILURE);
+ usage(argv[0], stderr, EXIT_FAILURE);
break;
}
} while (name != -1);
--
@@ -706,193 +706,194 @@ static unsigned int conf_ip6(unsigned int ifi,
/**
* usage() - Print usage, exit with given status code
* @name: Executable name
+ * @f: Stream to print usage info to
* @status: Status code for exit()
*/
-static void usage(const char *name, int status)
+static void usage(const char *name, FILE *f, int status)
{
if (strstr(name, "pasta")) {
- info("Usage: %s [OPTION]... [COMMAND] [ARGS]...", name);
- info(" %s [OPTION]... PID", name);
- info(" %s [OPTION]... --netns [PATH|NAME]", name);
- info("");
- info("Without PID or --netns, run the given command or a");
- info("default shell in a new network and user namespace, and");
- info("connect it via pasta.");
+ fprintf(f, "Usage: %s [OPTION]... [COMMAND] [ARGS]...\n", name);
+ fprintf(f, " %s [OPTION]... PID\n", name);
+ fprintf(f, " %s [OPTION]... --netns [PATH|NAME]\n", name);
+ fprintf(f, "\n");
+ fprintf(f, "Without PID or --netns, run the given command or a\n");
+ fprintf(f, "default shell in a new network and user namespace, and\n");
+ fprintf(f, "connect it via pasta.\n");
} else {
- info("Usage: %s [OPTION]...", name);
+ fprintf(f, "Usage: %s [OPTION]...\n", name);
}
- info("");
-
-
- info( " -d, --debug Be verbose");
- info( " --trace Be extra verbose, implies --debug");
- info( " -q, --quiet Don't print informational messages");
- info( " -f, --foreground Don't run in background");
- info( " default: run in background if started from a TTY");
- info( " -e, --stderr Log to stderr too");
- info( " default: log to system logger only if started from a TTY");
- info( " -l, --log-file PATH Log (only) to given file");
- info( " --log-size BYTES Maximum size of log file");
- info( " default: 1 MiB");
- info( " --runas UID|UID:GID Run as given UID, GID, which can be");
- info( " numeric, or login and group names");
- info( " default: drop to user \"nobody\"");
- info( " -h, --help Display this help message and exit");
- info( " --version Show version and exit");
+ fprintf(f, "\n");
+
+
+ fprintf(f, " -d, --debug Be verbose\n");
+ fprintf(f, " --trace Be extra verbose, implies --debug\n");
+ fprintf(f, " -q, --quiet Don't print informational messages\n");
+ fprintf(f, " -f, --foreground Don't run in background\n");
+ fprintf(f, " default: run in background if started from a TTY\n");
+ fprintf(f, " -e, --stderr Log to stderr too\n");
+ fprintf(f, " default: log to system logger only if started from a TTY\n");
+ fprintf(f, " -l, --log-file PATH Log (only) to given file\n");
+ fprintf(f, " --log-size BYTES Maximum size of log file\n");
+ fprintf(f, " default: 1 MiB\n");
+ fprintf(f, " --runas UID|UID:GID Run as given UID, GID, which can be\n");
+ fprintf(f, " numeric, or login and group names\n");
+ fprintf(f, " default: drop to user \"nobody\"\n");
+ fprintf(f, " -h, --help Display this help message and exit\n");
+ fprintf(f, " --version Show version and exit\n");
if (strstr(name, "pasta")) {
- info( " -I, --ns-ifname NAME namespace interface name");
- info( " default: same interface name as external one");
+ fprintf(f, " -I, --ns-ifname NAME namespace interface name\n");
+ fprintf(f, " default: same interface name as external one\n");
} else {
- info( " -s, --socket PATH UNIX domain socket path");
- info( " default: probe free path starting from "
- UNIX_SOCK_PATH, 1);
+ fprintf(f, " -s, --socket PATH UNIX domain socket path\n");
+ fprintf(f, " default: probe free path starting from "
+ UNIX_SOCK_PATH "\n", 1);
}
- info( " -F, --fd FD Use FD as pre-opened connected socket");
- info( " -p, --pcap FILE Log tap-facing traffic to pcap file");
- info( " -P, --pid FILE Write own PID to the given file");
- info( " -m, --mtu MTU Assign MTU via DHCP/NDP");
- info( " a zero value disables assignment");
- info( " default: 65520: maximum 802.3 MTU minus 802.3 header");
- info( " length, rounded to 32 bits (IPv4 words)");
- info( " -a, --address ADDR Assign IPv4 or IPv6 address ADDR");
- info( " can be specified zero to two times (for IPv4 and IPv6)");
- info( " default: use addresses from interface with default route");
- info( " -n, --netmask MASK Assign IPv4 MASK, dot-decimal or bits");
- info( " default: netmask from matching address on the host");
- info( " -M, --mac-addr ADDR Use source MAC address ADDR");
- info( " default: MAC address from interface with default route");
- info( " -g, --gateway ADDR Pass IPv4 or IPv6 address as gateway");
- info( " default: gateway from interface with default route");
- info( " -i, --interface NAME Interface for addresses and routes");
- info( " default: from --outbound-if4 and --outbound-if6, if any");
- info( " otherwise interface with first default route");
- info( " -o, --outbound ADDR Bind to address as outbound source");
- info( " can be specified zero to two times (for IPv4 and IPv6)");
- info( " default: use source address from routing tables");
- info( " --outbound-if4 NAME Bind to outbound interface for IPv4");
- info( " default: use interface from default route");
- info( " --outbound-if6 NAME Bind to outbound interface for IPv6");
- info( " default: use interface from default route");
- info( " -D, --dns ADDR Use IPv4 or IPv6 address as DNS");
- info( " can be specified multiple times");
- info( " a single, empty option disables DNS information");
+ fprintf(f, " -F, --fd FD Use FD as pre-opened connected socket\n");
+ fprintf(f, " -p, --pcap FILE Log tap-facing traffic to pcap file\n");
+ fprintf(f, " -P, --pid FILE Write own PID to the given file\n");
+ fprintf(f, " -m, --mtu MTU Assign MTU via DHCP/NDP\n");
+ fprintf(f, " a zero value disables assignment\n");
+ fprintf(f, " default: 65520: maximum 802.3 MTU minus 802.3 header\n");
+ fprintf(f, " length, rounded to 32 bits (IPv4 words)\n");
+ fprintf(f, " -a, --address ADDR Assign IPv4 or IPv6 address ADDR\n");
+ fprintf(f, " can be specified zero to two times (for IPv4 and IPv6)\n");
+ fprintf(f, " default: use addresses from interface with default route\n");
+ fprintf(f, " -n, --netmask MASK Assign IPv4 MASK, dot-decimal or bits\n");
+ fprintf(f, " default: netmask from matching address on the host\n");
+ fprintf(f, " -M, --mac-addr ADDR Use source MAC address ADDR\n");
+ fprintf(f, " default: MAC address from interface with default route\n");
+ fprintf(f, " -g, --gateway ADDR Pass IPv4 or IPv6 address as gateway\n");
+ fprintf(f, " default: gateway from interface with default route\n");
+ fprintf(f, " -i, --interface NAME Interface for addresses and routes\n");
+ fprintf(f, " default: from --outbound-if4 and --outbound-if6, if any\n");
+ fprintf(f, " otherwise interface with first default route\n");
+ fprintf(f, " -o, --outbound ADDR Bind to address as outbound source\n");
+ fprintf(f, " can be specified zero to two times (for IPv4 and IPv6)\n");
+ fprintf(f, " default: use source address from routing tables\n");
+ fprintf(f, " --outbound-if4 NAME Bind to outbound interface for IPv4\n");
+ fprintf(f, " default: use interface from default route\n");
+ fprintf(f, " --outbound-if6 NAME Bind to outbound interface for IPv6\n");
+ fprintf(f, " default: use interface from default route\n");
+ fprintf(f, " -D, --dns ADDR Use IPv4 or IPv6 address as DNS\n");
+ fprintf(f, " can be specified multiple times\n");
+ fprintf(f, " a single, empty option disables DNS information\n");
if (strstr(name, "pasta"))
- info( " default: don't use any addresses");
+ fprintf(f, " default: don't use any addresses\n");
else
- info( " default: use addresses from /etc/resolv.conf");
+ fprintf(f, " default: use addresses from /etc/resolv.conf\n");
- info( " -S, --search LIST Space-separated list, search domains");
- info( " a single, empty option disables the DNS search list");
+ fprintf(f, " -S, --search LIST Space-separated list, search domains\n");
+ fprintf(f, " a single, empty option disables the DNS search list\n");
if (strstr(name, "pasta"))
- info( " default: don't use any search list");
+ fprintf(f, " default: don't use any search list\n");
else
- info( " default: use search list from /etc/resolv.conf");
+ fprintf(f, " default: use search list from /etc/resolv.conf\n");
if (strstr(name, "pasta"))
- info(" --dhcp-dns \tPass DNS list via DHCP/DHCPv6/NDP");
+ fprintf(f, " --dhcp-dns \tPass DNS list via DHCP/DHCPv6/NDP\n");
else
- info(" --no-dhcp-dns No DNS list in DHCP/DHCPv6/NDP");
+ fprintf(f, " --no-dhcp-dns No DNS list in DHCP/DHCPv6/NDP\n");
if (strstr(name, "pasta"))
- info(" --dhcp-search Pass list via DHCP/DHCPv6/NDP");
+ fprintf(f, " --dhcp-search Pass list via DHCP/DHCPv6/NDP\n");
else
- info(" --no-dhcp-search No list in DHCP/DHCPv6/NDP");
-
- info( " --dns-forward ADDR Forward DNS queries sent to ADDR");
- info( " can be specified zero to two times (for IPv4 and IPv6)");
- info( " default: don't forward DNS queries");
-
- info( " --no-tcp Disable TCP protocol handler");
- info( " --no-udp Disable UDP protocol handler");
- info( " --no-icmp Disable ICMP/ICMPv6 protocol handler");
- info( " --no-dhcp Disable DHCP server");
- info( " --no-ndp Disable NDP responses");
- info( " --no-dhcpv6 Disable DHCPv6 server");
- info( " --no-ra Disable router advertisements");
- info( " --no-map-gw Don't map gateway address to host");
- info( " -4, --ipv4-only Enable IPv4 operation only");
- info( " -6, --ipv6-only Enable IPv6 operation only");
+ fprintf(f, " --no-dhcp-search No list in DHCP/DHCPv6/NDP\n");
+
+ fprintf(f, " --dns-forward ADDR Forward DNS queries sent to ADDR\n");
+ fprintf(f, " can be specified zero to two times (for IPv4 and IPv6)\n");
+ fprintf(f, " default: don't forward DNS queries\n");
+
+ fprintf(f, " --no-tcp Disable TCP protocol handler\n");
+ fprintf(f, " --no-udp Disable UDP protocol handler\n");
+ fprintf(f, " --no-icmp Disable ICMP/ICMPv6 protocol handler\n");
+ fprintf(f, " --no-dhcp Disable DHCP server\n");
+ fprintf(f, " --no-ndp Disable NDP responses\n");
+ fprintf(f, " --no-dhcpv6 Disable DHCPv6 server\n");
+ fprintf(f, " --no-ra Disable router advertisements\n");
+ fprintf(f, " --no-map-gw Don't map gateway address to host\n");
+ fprintf(f, " -4, --ipv4-only Enable IPv4 operation only\n");
+ fprintf(f, " -6, --ipv6-only Enable IPv6 operation only\n");
if (strstr(name, "pasta"))
goto pasta_opts;
- info( " -1, --one-off Quit after handling one single client");
- info( " -t, --tcp-ports SPEC TCP port forwarding to guest");
- info( " can be specified multiple times");
- info( " SPEC can be:");
- info( " 'none': don't forward any ports");
- info( " 'all': forward all unbound, non-ephemeral ports");
- info( " a comma-separated list, optionally ranged with '-'");
- info( " and optional target ports after ':', with optional");
- info( " address specification suffixed by '/' and optional");
- info( " interface prefixed by '%%'. Ranges can be reduced by");
- info( " excluding ports or ranges prefixed by '~'");
- info( " Examples:");
- info( " -t 22 Forward local port 22 to 22 on guest");
- info( " -t 22:23 Forward local port 22 to 23 on guest");
- info( " -t 22,25 Forward ports 22, 25 to ports 22, 25");
- info( " -t 22-80 Forward ports 22 to 80");
- info( " -t 22-80:32-90 Forward ports 22 to 80 to");
- info( " corresponding port numbers plus 10");
- info( " -t 192.0.2.1/5 Bind port 5 of 192.0.2.1 to guest");
- info( " -t 5-25,~10-20 Forward ports 5 to 9, and 21 to 25");
- info( " -t ~25 Forward all ports except for 25");
- info( " default: none");
- info( " -u, --udp-ports SPEC UDP port forwarding to guest");
- info( " SPEC is as described for TCP above");
- info( " default: none");
+ fprintf(f, " -1, --one-off Quit after handling one single client\n");
+ fprintf(f, " -t, --tcp-ports SPEC TCP port forwarding to guest\n");
+ fprintf(f, " can be specified multiple times\n");
+ fprintf(f, " SPEC can be:\n");
+ fprintf(f, " 'none': don't forward any ports\n");
+ fprintf(f, " 'all': forward all unbound, non-ephemeral ports\n");
+ fprintf(f, " a comma-separated list, optionally ranged with '-'\n");
+ fprintf(f, " and optional target ports after ':', with optional\n");
+ fprintf(f, " address specification suffixed by '/' and optional\n");
+ fprintf(f, " interface prefixed by '%%'. Ranges can be reduced by\n");
+ fprintf(f, " excluding ports or ranges prefixed by '~'\n");
+ fprintf(f, " Examples:\n");
+ fprintf(f, " -t 22 Forward local port 22 to 22 on guest\n");
+ fprintf(f, " -t 22:23 Forward local port 22 to 23 on guest\n");
+ fprintf(f, " -t 22,25 Forward ports 22, 25 to ports 22, 25\n");
+ fprintf(f, " -t 22-80 Forward ports 22 to 80\n");
+ fprintf(f, " -t 22-80:32-90 Forward ports 22 to 80 to\n");
+ fprintf(f, " corresponding port numbers plus 10\n");
+ fprintf(f, " -t 192.0.2.1/5 Bind port 5 of 192.0.2.1 to guest\n");
+ fprintf(f, " -t 5-25,~10-20 Forward ports 5 to 9, and 21 to 25\n");
+ fprintf(f, " -t ~25 Forward all ports except for 25\n");
+ fprintf(f, " default: none\n");
+ fprintf(f, " -u, --udp-ports SPEC UDP port forwarding to guest\n");
+ fprintf(f, " SPEC is as described for TCP above\n");
+ fprintf(f, " default: none\n");
exit(status);
pasta_opts:
- info( " -t, --tcp-ports SPEC TCP port forwarding to namespace");
- info( " can be specified multiple times");
- info( " SPEC can be:");
- info( " 'none': don't forward any ports");
- info( " 'auto': forward all ports currently bound in namespace");
- info( " a comma-separated list, optionally ranged with '-'");
- info( " and optional target ports after ':', with optional");
- info( " address specification suffixed by '/' and optional");
- info( " interface prefixed by '%%'. Examples:");
- info( " -t 22 Forward local port 22 to port 22 in netns");
- info( " -t 22:23 Forward local port 22 to port 23");
- info( " -t 22,25 Forward ports 22, 25 to ports 22, 25");
- info( " -t 22-80 Forward ports 22 to 80");
- info( " -t 22-80:32-90 Forward ports 22 to 80 to");
- info( " corresponding port numbers plus 10");
- info( " -t 192.0.2.1/5 Bind port 5 of 192.0.2.1 to namespace");
- info( " -t 5-25,~10-20 Forward ports 5 to 9, and 21 to 25");
- info( " -t ~25 Forward all bound ports except for 25");
- info( " default: auto");
- info( " IPv6 bound ports are also forwarded for IPv4");
- info( " -u, --udp-ports SPEC UDP port forwarding to namespace");
- info( " SPEC is as described for TCP above");
- info( " default: auto");
- info( " IPv6 bound ports are also forwarded for IPv4");
- info( " unless specified, with '-t auto', UDP ports with numbers");
- info( " corresponding to forwarded TCP port numbers are");
- info( " forwarded too");
- info( " -T, --tcp-ns SPEC TCP port forwarding to init namespace");
- info( " SPEC is as described above");
- info( " default: auto");
- info( " -U, --udp-ns SPEC UDP port forwarding to init namespace");
- info( " SPEC is as described above");
- info( " default: auto");
- info( " --userns NSPATH Target user namespace to join");
- info( " --netns PATH|NAME Target network namespace to join");
- info( " --netns-only Don't join existing user namespace");
- info( " implied if PATH or NAME are given without --userns");
- info( " --no-netns-quit Don't quit if filesystem-bound target");
- info( " network namespace is deleted");
- info( " --config-net Configure tap interface in namespace");
- info( " --no-copy-routes DEPRECATED:");
- info( " Don't copy all routes to namespace");
- info( " --no-copy-addrs DEPRECATED:");
- info( " Don't copy all addresses to namespace");
- info( " --ns-mac-addr ADDR Set MAC address on tap interface");
+ fprintf(f, " -t, --tcp-ports SPEC TCP port forwarding to namespace\n");
+ fprintf(f, " can be specified multiple times\n");
+ fprintf(f, " SPEC can be:\n");
+ fprintf(f, " 'none': don't forward any ports\n");
+ fprintf(f, " 'auto': forward all ports currently bound in namespace\n");
+ fprintf(f, " a comma-separated list, optionally ranged with '-'\n");
+ fprintf(f, " and optional target ports after ':', with optional\n");
+ fprintf(f, " address specification suffixed by '/' and optional\n");
+ fprintf(f, " interface prefixed by '%%'. Examples:\n");
+ fprintf(f, " -t 22 Forward local port 22 to port 22 in netns\n");
+ fprintf(f, " -t 22:23 Forward local port 22 to port 23\n");
+ fprintf(f, " -t 22,25 Forward ports 22, 25 to ports 22, 25\n");
+ fprintf(f, " -t 22-80 Forward ports 22 to 80\n");
+ fprintf(f, " -t 22-80:32-90 Forward ports 22 to 80 to\n");
+ fprintf(f, " corresponding port numbers plus 10\n");
+ fprintf(f, " -t 192.0.2.1/5 Bind port 5 of 192.0.2.1 to namespace\n");
+ fprintf(f, " -t 5-25,~10-20 Forward ports 5 to 9, and 21 to 25\n");
+ fprintf(f, " -t ~25 Forward all bound ports except for 25\n");
+ fprintf(f, " default: auto\n");
+ fprintf(f, " IPv6 bound ports are also forwarded for IPv4\n");
+ fprintf(f, " -u, --udp-ports SPEC UDP port forwarding to namespace\n");
+ fprintf(f, " SPEC is as described for TCP above\n");
+ fprintf(f, " default: auto\n");
+ fprintf(f, " IPv6 bound ports are also forwarded for IPv4\n");
+ fprintf(f, " unless specified, with '-t auto', UDP ports with numbers\n");
+ fprintf(f, " corresponding to forwarded TCP port numbers are\n");
+ fprintf(f, " forwarded too\n");
+ fprintf(f, " -T, --tcp-ns SPEC TCP port forwarding to init namespace\n");
+ fprintf(f, " SPEC is as described above\n");
+ fprintf(f, " default: auto\n");
+ fprintf(f, " -U, --udp-ns SPEC UDP port forwarding to init namespace\n");
+ fprintf(f, " SPEC is as described above\n");
+ fprintf(f, " default: auto\n");
+ fprintf(f, " --userns NSPATH Target user namespace to join\n");
+ fprintf(f, " --netns PATH|NAME Target network namespace to join\n");
+ fprintf(f, " --netns-only Don't join existing user namespace\n");
+ fprintf(f, " implied if PATH or NAME are given without --userns\n");
+ fprintf(f, " --no-netns-quit Don't quit if filesystem-bound target\n");
+ fprintf(f, " network namespace is deleted\n");
+ fprintf(f, " --config-net Configure tap interface in namespace\n");
+ fprintf(f, " --no-copy-routes DEPRECATED:\n");
+ fprintf(f, " Don't copy all routes to namespace\n");
+ fprintf(f, " --no-copy-addrs DEPRECATED:\n");
+ fprintf(f, " Don't copy all addresses to namespace\n");
+ fprintf(f, " --ns-mac-addr ADDR Set MAC address on tap interface\n");
exit(status);
}
@@ -1637,12 +1638,11 @@ void conf(struct ctx *c, int argc, char **argv)
/* Handle these later, once addresses are configured */
break;
case 'h':
- log_to_stdout = 1;
- usage(argv[0], EXIT_SUCCESS);
+ usage(argv[0], stdout, EXIT_SUCCESS);
break;
case '?':
default:
- usage(argv[0], EXIT_FAILURE);
+ usage(argv[0], stderr, EXIT_FAILURE);
break;
}
} while (name != -1);
--
2.45.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] log: Remove log_to_stdout option
2024-05-29 9:04 [PATCH 0/3] conf, log: Fix regression in usage() printing David Gibson
2024-05-29 9:04 ` [PATCH 1/3] conf: Remove unhelpful usage() wrapper David Gibson
2024-05-29 9:04 ` [PATCH 2/3] conf: Don't print usage via the logging subsystem David Gibson
@ 2024-05-29 9:04 ` David Gibson
2 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2024-05-29 9:04 UTC (permalink / raw)
To: passt-dev, Stefano Brivio; +Cc: erik.sjolund, David Gibson
Now that we've simplified how usage() works, nothing ever sets the
log_to_stdout flag. Eliminate it.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
log.c | 8 +++-----
log.h | 1 -
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/log.c b/log.c
index e3630c35..aaf2beb5 100644
--- a/log.c
+++ b/log.c
@@ -46,18 +46,16 @@ static char log_header[BUFSIZ]; /* File header, written back on cuts */
static time_t log_start; /* Start timestamp */
int log_trace; /* --trace mode enabled */
-int log_to_stdout; /* Print to stdout instead of stderr */
void vlogmsg(int pri, const char *format, va_list ap)
{
bool debug_print = (log_mask & LOG_MASK(LOG_DEBUG)) && log_file == -1;
bool early_print = LOG_PRI(log_mask) == LOG_EARLY;
- FILE *out = log_to_stdout ? stdout : stderr;
struct timespec tp;
if (debug_print) {
clock_gettime(CLOCK_REALTIME, &tp);
- fprintf(out, "%lli.%04lli: ",
+ fprintf(stderr, "%lli.%04lli: ",
(long long int)tp.tv_sec - log_start,
(long long int)tp.tv_nsec / (100L * 1000));
}
@@ -75,9 +73,9 @@ void vlogmsg(int pri, const char *format, va_list ap)
}
if (debug_print || (early_print && !(log_opt & LOG_PERROR))) {
- (void)vfprintf(out, format, ap);
+ (void)vfprintf(stderr, format, ap);
if (format[strlen(format)] != '\n')
- fprintf(out, "\n");
+ fprintf(stderr, "\n");
}
}
diff --git a/log.h b/log.h
index 9c38182f..e0aab5a9 100644
--- a/log.h
+++ b/log.h
@@ -28,7 +28,6 @@ void logmsg(int pri, const char *format, ...)
} while (0)
extern int log_trace;
-extern int log_to_stdout;
void trace_init(int enable);
#define trace(...) \
do { \
--
@@ -28,7 +28,6 @@ void logmsg(int pri, const char *format, ...)
} while (0)
extern int log_trace;
-extern int log_to_stdout;
void trace_init(int enable);
#define trace(...) \
do { \
--
2.45.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] conf: Don't print usage via the logging subsystem
2024-05-29 9:04 ` [PATCH 2/3] conf: Don't print usage via the logging subsystem David Gibson
@ 2024-06-04 22:40 ` Stefano Brivio
2024-06-04 23:51 ` David Gibson
0 siblings, 1 reply; 6+ messages in thread
From: Stefano Brivio @ 2024-06-04 22:40 UTC (permalink / raw)
To: David Gibson; +Cc: passt-dev, erik.sjolund
On Wed, 29 May 2024 19:04:04 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:
> The message from usage() when given invalid options, or the -h / --help
> option is currently printed by many calls to the info() function, also
> used for runtime logging of informational messages.
>
> That isn't useful: the usage message should always go to the terminal
> (stdout or stderr), never syslog or a logfile. It should never be
> filtered by priority. Really the only thing using the common logging
> functions does is give more opportunities for something to go wrong.
>
> Replace all the info() calls with direct fprintf() calls. This does mean
> manually adding "\n" to each message. A little messy, but worth it for the
> simplicity in other dimensions.
Yes, definitely less messy than the existing implementation, I just
wonder:
> Link: https://bugs.passt.top/show_bug.cgi?id=90
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> conf.c | 318 ++++++++++++++++++++++++++++-----------------------------
> 1 file changed, 159 insertions(+), 159 deletions(-)
>
> diff --git a/conf.c b/conf.c
> index f2a92574..31f5b197 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -706,193 +706,194 @@ static unsigned int conf_ip6(unsigned int ifi,
> /**
> * usage() - Print usage, exit with given status code
> * @name: Executable name
> + * @f: Stream to print usage info to
> * @status: Status code for exit()
> */
> -static void usage(const char *name, int status)
> +static void usage(const char *name, FILE *f, int status)
> {
> if (strstr(name, "pasta")) {
> - info("Usage: %s [OPTION]... [COMMAND] [ARGS]...", name);
> - info(" %s [OPTION]... PID", name);
> - info(" %s [OPTION]... --netns [PATH|NAME]", name);
> - info("");
> - info("Without PID or --netns, run the given command or a");
> - info("default shell in a new network and user namespace, and");
> - info("connect it via pasta.");
> + fprintf(f, "Usage: %s [OPTION]... [COMMAND] [ARGS]...\n", name);
> + fprintf(f, " %s [OPTION]... PID\n", name);
> + fprintf(f, " %s [OPTION]... --netns [PATH|NAME]\n", name);
> + fprintf(f, "\n");
> + fprintf(f, "Without PID or --netns, run the given command or a\n");
> + fprintf(f, "default shell in a new network and user namespace, and\n");
> + fprintf(f, "connect it via pasta.\n");
I haven't checked how it looks like in the end, but in most of this
function, we use fprintf() without arguments after the format, and we
need explicit newlines anyway, so, what if we concatenate the whole
output, say:
fprintf(f,
"Without PID or --netns, run the given command or a\n"
"default shell in a new network and user namespace, and\n"
"connect it via pasta.\n"
);
?
I used separate info() calls (or whatever they were) in the past
just because of the convenient newlines.
--
Stefano
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] conf: Don't print usage via the logging subsystem
2024-06-04 22:40 ` Stefano Brivio
@ 2024-06-04 23:51 ` David Gibson
0 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2024-06-04 23:51 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev, erik.sjolund
[-- Attachment #1: Type: text/plain, Size: 3369 bytes --]
On Wed, Jun 05, 2024 at 12:40:41AM +0200, Stefano Brivio wrote:
> On Wed, 29 May 2024 19:04:04 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
>
> > The message from usage() when given invalid options, or the -h / --help
> > option is currently printed by many calls to the info() function, also
> > used for runtime logging of informational messages.
> >
> > That isn't useful: the usage message should always go to the terminal
> > (stdout or stderr), never syslog or a logfile. It should never be
> > filtered by priority. Really the only thing using the common logging
> > functions does is give more opportunities for something to go wrong.
> >
> > Replace all the info() calls with direct fprintf() calls. This does mean
> > manually adding "\n" to each message. A little messy, but worth it for the
> > simplicity in other dimensions.
>
> Yes, definitely less messy than the existing implementation, I just
> wonder:
>
> > Link: https://bugs.passt.top/show_bug.cgi?id=90
> >
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> > conf.c | 318 ++++++++++++++++++++++++++++-----------------------------
> > 1 file changed, 159 insertions(+), 159 deletions(-)
> >
> > diff --git a/conf.c b/conf.c
> > index f2a92574..31f5b197 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -706,193 +706,194 @@ static unsigned int conf_ip6(unsigned int ifi,
> > /**
> > * usage() - Print usage, exit with given status code
> > * @name: Executable name
> > + * @f: Stream to print usage info to
> > * @status: Status code for exit()
> > */
> > -static void usage(const char *name, int status)
> > +static void usage(const char *name, FILE *f, int status)
> > {
> > if (strstr(name, "pasta")) {
> > - info("Usage: %s [OPTION]... [COMMAND] [ARGS]...", name);
> > - info(" %s [OPTION]... PID", name);
> > - info(" %s [OPTION]... --netns [PATH|NAME]", name);
> > - info("");
> > - info("Without PID or --netns, run the given command or a");
> > - info("default shell in a new network and user namespace, and");
> > - info("connect it via pasta.");
> > + fprintf(f, "Usage: %s [OPTION]... [COMMAND] [ARGS]...\n", name);
> > + fprintf(f, " %s [OPTION]... PID\n", name);
> > + fprintf(f, " %s [OPTION]... --netns [PATH|NAME]\n", name);
> > + fprintf(f, "\n");
> > + fprintf(f, "Without PID or --netns, run the given command or a\n");
> > + fprintf(f, "default shell in a new network and user namespace, and\n");
> > + fprintf(f, "connect it via pasta.\n");
>
> I haven't checked how it looks like in the end, but in most of this
> function, we use fprintf() without arguments after the format, and we
> need explicit newlines anyway, so, what if we concatenate the whole
> output, say:
>
> fprintf(f,
> "Without PID or --netns, run the given command or a\n"
> "default shell in a new network and user namespace, and\n"
> "connect it via pasta.\n"
> );
>
> ?
Fair point, I've made that change.
> I used separate info() calls (or whatever they were) in the past
> just because of the convenient newlines.
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-06-05 0:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-29 9:04 [PATCH 0/3] conf, log: Fix regression in usage() printing David Gibson
2024-05-29 9:04 ` [PATCH 1/3] conf: Remove unhelpful usage() wrapper David Gibson
2024-05-29 9:04 ` [PATCH 2/3] conf: Don't print usage via the logging subsystem David Gibson
2024-06-04 22:40 ` Stefano Brivio
2024-06-04 23:51 ` David Gibson
2024-05-29 9:04 ` [PATCH 3/3] log: Remove log_to_stdout option David Gibson
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).