diff mbox series

[rt-tests,v3,04/33] rt-util: Add rt_init function

Message ID 20210320183829.1318-5-dwagner@suse.de
State New
Headers show
Series JSON cleanups and more tests updated | expand

Commit Message

Daniel Wagner March 20, 2021, 6:38 p.m. UTC 
rt_init() should be called as first in the test program.
The main job for this function is to copy the command line
before getopt() runs. By default, getopt_long() permutes
the contents of argv as it scans, so that eventually all the
nonoptions are at the end. This is confusing in the JSON
output, thus copy the command line before we call getopt_long().

Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 src/include/rt-utils.h |  2 ++
 src/lib/rt-utils.c     | 47 +++++++++++++++++++++++++++++++++++-------
 2 files changed, 42 insertions(+), 7 deletions(-)

Comments

John Kacur May 7, 2021, 4:27 p.m. UTC | #1 
On Sat, 20 Mar 2021, Daniel Wagner wrote:

> rt_init() should be called as first in the test program.

> The main job for this function is to copy the command line

> before getopt() runs. By default, getopt_long() permutes

> the contents of argv as it scans, so that eventually all the

> nonoptions are at the end. This is confusing in the JSON

> output, thus copy the command line before we call getopt_long().

> 

> Signed-off-by: Daniel Wagner <dwagner@suse.de>

> ---

>  src/include/rt-utils.h |  2 ++

>  src/lib/rt-utils.c     | 47 +++++++++++++++++++++++++++++++++++-------

>  2 files changed, 42 insertions(+), 7 deletions(-)

> 

> diff --git a/src/include/rt-utils.h b/src/include/rt-utils.h

> index 36af92b170df..1dbd3f1dd8e3 100644

> --- a/src/include/rt-utils.h

> +++ b/src/include/rt-utils.h

> @@ -80,6 +80,8 @@ static inline int64_t calctime(struct timespec t)

>  	return time;

>  }

>  

> +void rt_init(int argc, char *argv[]);

> +

>  void rt_write_json(const char *filename, int argc, char *argv[],

>  		   void (*cb)(FILE *, void *),

>  		   void *data);

> diff --git a/src/lib/rt-utils.c b/src/lib/rt-utils.c

> index 00907c573d6a..8f0e0943b137 100644

> --- a/src/lib/rt-utils.c

> +++ b/src/lib/rt-utils.c

> @@ -29,12 +29,15 @@

>  #include "rt-error.h"

>  

>  #define  TRACEBUFSIZ  1024

> +#define  MAX_COMMAND_LINE 4096

>  

>  static char debugfileprefix[MAX_PATH];

>  static char *fileprefix;

>  static int trace_fd = -1;

>  static int tracemark_fd = -1;

>  static __thread char tracebuf[TRACEBUFSIZ];

> +static char test_cmdline[MAX_COMMAND_LINE];

> +static int rt_init_run;

>  

>  /*

>   * Finds the tracing directory in a mounted debugfs

> @@ -486,6 +489,34 @@ void disable_trace_mark(void)

>  	close_tracemark_fd();

>  }

>  

> +void rt_init(int argc, char *argv[])

> +{

> +	int offset = 0;

> +	int len, i;

> +

> +	test_cmdline[0] = '\0';

> +

> +	/*

> +	 * getopt_long() permutes the contents of argv as it scans, so

> +	 * that eventually all the nonoptions are at the end. Make a

> +	 * copy before calling getopt_long().

> +	 */

> +	for (i = 0; i < argc;) {

> +		len = strlen(argv[i]);

> +		if (offset + len + 1 >= MAX_COMMAND_LINE)

> +			break;

> +

> +		strcat(test_cmdline, argv[i]);

> +		i++;

> +		if (i < argc)

> +			strcat(test_cmdline, " ");

> +

> +		offset += len + 1;

> +	}

> +

> +	rt_init_run = 1;

> +}

> +

>  static char *get_cmdline(int argc, char *argv[])

>  {

>  	char *cmdline;

> @@ -519,7 +550,7 @@ void rt_write_json(const char *filename, int argc, char *argv[],

>  	struct timeval tv;

>  	char tsbuf[64];

>  	struct tm *tm;

> -	char *cmdline;

> +	char *cmdline = NULL;

>  	FILE *f, *s;

>  	time_t t;

>  	size_t n;

> @@ -533,10 +564,11 @@ void rt_write_json(const char *filename, int argc, char *argv[],

>  			err_exit(errno, "Failed to open '%s'\n", filename);

>  	}

>  

> -	cmdline = get_cmdline(argc, argv);

> -	if (!cmdline)

> -		err_exit(ENOMEM, "get_cmdline()");

> -

> +	if (!rt_init_run) {

> +		cmdline = get_cmdline(argc, argv);

> +		if (!cmdline)

> +			err_exit(ENOMEM, "get_cmdline()");

> +	}

>  

>  	gettimeofday(&tv, NULL);

>  	t = tv.tv_sec;

> @@ -557,7 +589,7 @@ void rt_write_json(const char *filename, int argc, char *argv[],

>  

>  	fprintf(f, "{\n");

>  	fprintf(f, "  \"file_version\": 1,\n");

> -	fprintf(f, "  \"cmdline:\": \"%s\",\n", cmdline);

> +	fprintf(f, "  \"cmdline:\": \"%s\",\n", rt_init_run? test_cmdline : cmdline);

>  	fprintf(f, "  \"rt_test_version:\": \"%1.2f\",\n", VERSION);

>  	fprintf(f, "  \"finished\": \"%s\",\n", tsbuf);

>  	fprintf(f, "  \"sysinfo\": {\n");

> @@ -573,7 +605,8 @@ void rt_write_json(const char *filename, int argc, char *argv[],

>  

>  	fprintf(f, "}\n");

>  

> -	free(cmdline);

> +	if (!rt_init_run)

> +		free(cmdline);

>  

>  	if (!filename || strcmp("-", filename))

>  		fclose(f);

> -- 

> 2.30.2

> 

> 

Signed-off-by: John Kacur <jkacur@redhat.com>


But I don't like the name, it's not initing any test state, it's copying 
the command line, and the name should reflect that.
Daniel Wagner May 12, 2021, 7:30 a.m. UTC | #2 
On Fri, May 07, 2021 at 12:27:53PM -0400, John Kacur wrote:
> But I don't like the name, it's not initing any test state, it's copying 

> the command line, and the name should reflect that.


I went for rt_init() because rt_test_start() code was part of
it. But without this part we could name it something like:

   rt_{copy|save|store}_{cmdline|args}

What's your preference?
diff mbox series

Patch

diff --git a/src/include/rt-utils.h b/src/include/rt-utils.h
index 36af92b170df..1dbd3f1dd8e3 100644
--- a/src/include/rt-utils.h
+++ b/src/include/rt-utils.h
@@ -80,6 +80,8 @@  static inline int64_t calctime(struct timespec t)
 	return time;
 }
 
+void rt_init(int argc, char *argv[]);
+
 void rt_write_json(const char *filename, int argc, char *argv[],
 		   void (*cb)(FILE *, void *),
 		   void *data);
diff --git a/src/lib/rt-utils.c b/src/lib/rt-utils.c
index 00907c573d6a..8f0e0943b137 100644
--- a/src/lib/rt-utils.c
+++ b/src/lib/rt-utils.c
@@ -29,12 +29,15 @@ 
 #include "rt-error.h"
 
 #define  TRACEBUFSIZ  1024
+#define  MAX_COMMAND_LINE 4096
 
 static char debugfileprefix[MAX_PATH];
 static char *fileprefix;
 static int trace_fd = -1;
 static int tracemark_fd = -1;
 static __thread char tracebuf[TRACEBUFSIZ];
+static char test_cmdline[MAX_COMMAND_LINE];
+static int rt_init_run;
 
 /*
  * Finds the tracing directory in a mounted debugfs
@@ -486,6 +489,34 @@  void disable_trace_mark(void)
 	close_tracemark_fd();
 }
 
+void rt_init(int argc, char *argv[])
+{
+	int offset = 0;
+	int len, i;
+
+	test_cmdline[0] = '\0';
+
+	/*
+	 * getopt_long() permutes the contents of argv as it scans, so
+	 * that eventually all the nonoptions are at the end. Make a
+	 * copy before calling getopt_long().
+	 */
+	for (i = 0; i < argc;) {
+		len = strlen(argv[i]);
+		if (offset + len + 1 >= MAX_COMMAND_LINE)
+			break;
+
+		strcat(test_cmdline, argv[i]);
+		i++;
+		if (i < argc)
+			strcat(test_cmdline, " ");
+
+		offset += len + 1;
+	}
+
+	rt_init_run = 1;
+}
+
 static char *get_cmdline(int argc, char *argv[])
 {
 	char *cmdline;
@@ -519,7 +550,7 @@  void rt_write_json(const char *filename, int argc, char *argv[],
 	struct timeval tv;
 	char tsbuf[64];
 	struct tm *tm;
-	char *cmdline;
+	char *cmdline = NULL;
 	FILE *f, *s;
 	time_t t;
 	size_t n;
@@ -533,10 +564,11 @@  void rt_write_json(const char *filename, int argc, char *argv[],
 			err_exit(errno, "Failed to open '%s'\n", filename);
 	}
 
-	cmdline = get_cmdline(argc, argv);
-	if (!cmdline)
-		err_exit(ENOMEM, "get_cmdline()");
-
+	if (!rt_init_run) {
+		cmdline = get_cmdline(argc, argv);
+		if (!cmdline)
+			err_exit(ENOMEM, "get_cmdline()");
+	}
 
 	gettimeofday(&tv, NULL);
 	t = tv.tv_sec;
@@ -557,7 +589,7 @@  void rt_write_json(const char *filename, int argc, char *argv[],
 
 	fprintf(f, "{\n");
 	fprintf(f, "  \"file_version\": 1,\n");
-	fprintf(f, "  \"cmdline:\": \"%s\",\n", cmdline);
+	fprintf(f, "  \"cmdline:\": \"%s\",\n", rt_init_run? test_cmdline : cmdline);
 	fprintf(f, "  \"rt_test_version:\": \"%1.2f\",\n", VERSION);
 	fprintf(f, "  \"finished\": \"%s\",\n", tsbuf);
 	fprintf(f, "  \"sysinfo\": {\n");
@@ -573,7 +605,8 @@  void rt_write_json(const char *filename, int argc, char *argv[],
 
 	fprintf(f, "}\n");
 
-	free(cmdline);
+	if (!rt_init_run)
+		free(cmdline);
 
 	if (!filename || strcmp("-", filename))
 		fclose(f);