Re: [PATCH 6/8] conf, log, Makefile: Add versioning information

On Fri, 7 Oct 2022 17:57:43 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:

...
  On Fri, Oct 07, 2022 at 02:47:40AM +0200, Stefano
Brivio wrote:
  Now that we can log to file, this might start to
be relevant.   
 
 Uh... I dont' understand the connection here. 
Because we might have a pile of log files collected somewhere (or
logs shared by users) and this adds the version to the log file.

...
 
  Signed-off-by:
Stefano Brivio <sbrivio(a)redhat.com>
 ---
  Makefile |  3 +++
  conf.c   | 12 ++++++++++--
  log.c    |  4 ++--
  passt.1  |  4 ++++
  util.h   |  8 ++++++++
  5 files changed, 27 insertions(+), 4 deletions(-)
 
 diff --git a/Makefile b/Makefile
 index fafb024..f7ddb84 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -9,6 +9,8 @@
  # Copyright (c) 2021 Red Hat GmbH
  # Author: Stefano Brivio <sbrivio(a)redhat.com>
  
 +VERSION ?= $(shell git describe --tags HEAD || echo "unknown\ version")
 +
  RLIMIT_STACK_VAL := $(shell /bin/sh -c 'ulimit -s')
  ifeq ($(RLIMIT_STACK_VAL),unlimited)
  RLIMIT_STACK_VAL := 1024
 @@ -31,6 +33,7 @@ FLAGS += -DNETNS_RUN_DIR=\"/run/netns\"
  FLAGS += -DPASST_AUDIT_ARCH=AUDIT_ARCH_$(AUDIT_ARCH)
  FLAGS += -DRLIMIT_STACK_VAL=$(RLIMIT_STACK_VAL)
  FLAGS += -DARCH=\"$(TARGET_ARCH)\"
 +FLAGS += -DVERSION=\"$(VERSION)\"
  
  PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c icmp.c igmp.c \
  	isolation.c lineread.c log.c mld.c ndp.c netlink.c packet.c passt.c \
 diff --git a/conf.c b/conf.c
 index f22940b..4ec3153 100644
 --- a/conf.c
 +++ b/conf.c
 @@ -626,6 +626,8 @@ static void usage(const char *name)
  	}
  	info("");
  
 +   
 
 Extraneous blank line. 
Oops.

...
 
  +	info(  
"  -v, --version	Show version and exit");   
 
 I'd suggest just '--version'.  '-v' is "verbose" more
often than it is
 "version". 
Um, yes, I'll drop -v.

...
 
   	info(  
"  -d, --debug		Be verbose, don't run in background");
  	info(   "      --trace		Be extra verbose, implies --debug");
  	info(   "  -q, --quiet		Don't print informational messages");
 @@ -993,6 +995,7 @@ void conf(struct ctx *c, int argc, char **argv)
  {
  	int netns_only = 0;
  	struct option options[] = {
 +		{"version",	no_argument,		NULL,		'v' },
  		{"debug",	no_argument,		NULL,		'd' },
  		{"quiet",	no_argument,		NULL,		'q' },
  		{"foreground",	no_argument,		NULL,		'f' },
 @@ -1057,9 +1060,9 @@ void conf(struct ctx *c, int argc, char **argv)
  
  	if (c->mode == MODE_PASTA) {
  		c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
 -		optstring = "dqfel:hI:p:P:m:a:n:M:g:i:D:S:46t:u:T:U:";
 +		optstring = "vdqfel:hI:p:P:m:a:n:M:g:i:D:S:46t:u:T:U:";
  	} else {
 -		optstring = "dqfel:hs:p:P:m:a:n:M:g:i:D:S:46t:u:";
 +		optstring = "vdqfel:hs:p:P:m:a:n:M:g:i:D:S:46t:u:";
  	}
  
  	c->tcp.fwd_in.mode = c->tcp.fwd_out.mode = 0;
 @@ -1197,6 +1200,11 @@ void conf(struct ctx *c, int argc, char **argv)
  				usage(argv[0]);
  			}
  			break;
 +		case 'v':
 +			fprintf(stdout,
 +				c->mode == MODE_PASST ? "passt " : "pasta ");
 +			fprintf(stdout, VERSION_BLOB);
 +			exit(EXIT_SUCCESS);
  		case 'd':
  			if (c->debug) {
  				err("Multiple --debug options given");
 diff --git a/log.c b/log.c
 index 85b13fe..2b088c4 100644
 --- a/log.c
 +++ b/log.c
 @@ -172,7 +172,7 @@ void passt_vsyslog(int pri, const char *format, va_list ap)
  }
  
  /**
 - * logfile_init() - Open log file and write header with PID and path
 + * logfile_init() - Open log file and write header with PID, version, path
   * @name:	Identifier for header: passt or pasta
   * @path:	Path to log file
   * @size:	Maximum size of log file: log_cut_size is calculatd here
 @@ -196,7 +196,7 @@ void logfile_init(const char *name, const char *path, size_t size)
  
  	log_size = size ? size : LOGFILE_SIZE_DEFAULT;
  
 -	n = snprintf(log_header, sizeof(log_header), "%s: %s (%i)",
 +	n = snprintf(log_header, sizeof(log_header), "%s " VERSION ": %s
(%i)",
  		     name, exe, getpid());
  
  	if (write(log_file, log_header, n) <= 0 ||
 diff --git a/passt.1 b/passt.1
 index 64236b6..c63a439 100644
 --- a/passt.1
 +++ b/passt.1
 @@ -77,6 +77,10 @@ for performance reasons.
  
  .SH OPTIONS
  
 +.TP
 +.BR \-v ", " \-\-version
 +Show version and exit.
 +
  .TP
  .BR \-d ", " \-\-debug
  Be verbose, don't run in background, don't log to the system logger.
 diff --git a/util.h b/util.h
 index 1adbf04..e8f99b6 100644
 --- a/util.h
 +++ b/util.h
 @@ -6,6 +6,14 @@
  #ifndef UTIL_H
  #define UTIL_H
  
 +#define VERSION_BLOB							       \
 +	VERSION "\n"							       \
 +	"Copyright (C) 2020-2022 Red Hat\n"				       \   
 
 Fwiw, I believe Red Hat legal suggests simply "Copyright Red Hat".
 AIUI the "(C)" thing is legally meaningless (unlike the word
 "copyright" or the actual © symbol), and the years are unnecessary and
 tend to get out of date. 
I just checked coreutils and gcc, but I see both points, I'll change
this.

-- 
Stefano