Skip to contentSkip to conversation

rt-migrate-test: exit early if nr_runs is non-positive

PATCH Superseded Boris Egorov 4 participants 9 messages Nov 27, 2014 – Feb 17, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 rt-migrate-test: exit early if nr_runs is non-positive

Program will crash if nr_runs is 0 due to dividing by it in print_results(). Let's exit early instead.

---
 src/rt-migrate-test/rt-migrate-test.c | 5 +++++
 1 file changed, 5 insertions(+)
(quoted diff) +0 -0

b/src/rt-migrate-test/rt-migrate-test.c
index e3c7a09..876a122 100644

src/rt-migrate-test/rt-migrate-test.c +5 -0
@@ -465,6 +465,11 @@ int main (int argc, char **argv)
  	parse_options(argc, argv);
+	if (nr_runs <= 0) {
+		fprintf(stderr, "Warning, --loops argument is non-positive. Exiting.\n");
+		exit(-1);
+	}
+
 	signal(SIGINT, stop_log);
  	if (argc >= (optind + 1))
Signature · 1 line
2.1.3
#1 lore raw
Quoted 4 lines · * Boris Egorov | 2014-11-27 16:20:34 [+0600]:

Program will crash if nr_runs is 0 due to dividing by it in print_results(). Let's exit early instead.

Has this been dealt with? I can't see this applied. I would prefer doing the value check in parse_options() itself. And looking at it, it seems that run_interval is interval ignored. Or is there macro magic invovled?

Quoted 21 lines

---
src/rt-migrate-test/rt-migrate-test.c | 5 +++++

1 file changed, 5 insertions(+)
src/rt-migrate-test/rt-migrate-test.c +5 -0
index e3c7a09..876a122 100644
@@ -465,6 +465,11 @@ int main (int argc, char **argv)
 	parse_options(argc, argv);
+	if (nr_runs <= 0) {
+		fprintf(stderr, "Warning, --loops argument is non-positive. Exiting.\n");
+		exit(-1);
+	}
+
 	signal(SIGINT, stop_log);
 	if (argc >= (optind + 1))
Signature · 1 line
2.1.3

Sebastian

#2 lore raw

Looks like this patch is not applied yet.

I can move value check to parse_options and send another version of patch. As for macros - yes, I already mentioned them in debian bug report thread. There are three of them (INTERVAL, RUN_INTERVAL, PROGRESS_CHARS) which are improperly (IMO) used instead of corresponding lowercase variables. I thought these changes should go to another patch.

Quoted 36 lines · Sebastian Andrzej Siewior
Quoted 4 lines · * Boris Egorov | 2014-11-27 16:20:34 [+0600]:

Program will crash if nr_runs is 0 due to dividing by it in print_results(). Let's exit early instead.

Has this been dealt with? I can't see this applied. I would prefer doing the value check in parse_options() itself. And looking at it, it seems that run_interval is interval ignored. Or is there macro magic invovled?

Quoted 21 lines

---
src/rt-migrate-test/rt-migrate-test.c | 5 +++++

1 file changed, 5 insertions(+)
src/rt-migrate-test/rt-migrate-test.c +5 -0
index e3c7a09..876a122 100644
@@ -465,6 +465,11 @@ int main (int argc, char **argv)
 	parse_options(argc, argv);
+	if (nr_runs <= 0) {
+		fprintf(stderr, "Warning, --loops argument is non-positive. Exiting.\n");
+		exit(-1);
+	}
+
 	signal(SIGINT, stop_log);
 	if (argc >= (optind + 1))
Signature · 1 line
2.1.3

Sebastian

Signature · 2 lines
Best regards,
Boris Egorov
#3 lore raw

Sorry, PROGRESS_CHARS macro is used correctly, but INTERVAL and RUN_INTERVAL not. In my opinion, they should be defined and used once in static variable assigning, and nowhere else.

Quoted 46 lines · Boris Egorov

Looks like this patch is not applied yet.

I can move value check to parse_options and send another version of patch. As for macros - yes, I already mentioned them in debian bug report thread. There are three of them (INTERVAL, RUN_INTERVAL, PROGRESS_CHARS) which are improperly (IMO) used instead of corresponding lowercase variables. I thought these changes should go to another patch.

Quoted 36 lines · Sebastian Andrzej Siewior
Quoted 4 lines · * Boris Egorov | 2014-11-27 16:20:34 [+0600]:

Program will crash if nr_runs is 0 due to dividing by it in print_results(). Let's exit early instead.

Has this been dealt with? I can't see this applied. I would prefer doing the value check in parse_options() itself. And looking at it, it seems that run_interval is interval ignored. Or is there macro magic invovled?

Quoted 21 lines

---
src/rt-migrate-test/rt-migrate-test.c | 5 +++++

1 file changed, 5 insertions(+)
src/rt-migrate-test/rt-migrate-test.c +5 -0
index e3c7a09..876a122 100644
@@ -465,6 +465,11 @@ int main (int argc, char **argv)
 	parse_options(argc, argv);
+	if (nr_runs <= 0) {
+		fprintf(stderr, "Warning, --loops argument is non-positive. Exiting.\n");
+		exit(-1);
+	}
+
 	signal(SIGINT, stop_log);
 	if (argc >= (optind + 1))
Signature · 1 line
2.1.3

Sebastian

Signature · 2 lines
Best regards,
Boris Egorov
#4 lore raw
Quoted 43 lines · Boris Egorov

Sorry, PROGRESS_CHARS macro is used correctly, but INTERVAL and RUN_INTERVAL not. In my opinion, they should be defined and used once in static variable assigning, and nowhere else.

Quoted 38 lines · Boris Egorov

Looks like this patch is not applied yet.

I can move value check to parse_options and send another version of patch. As for macros - yes, I already mentioned them in debian bug report thread. There are three of them (INTERVAL, RUN_INTERVAL, PROGRESS_CHARS) which are improperly (IMO) used instead of corresponding lowercase variables. I thought these changes should go to another patch.

Quoted 29 lines · Sebastian Andrzej Siewior
Quoted 4 lines · * Boris Egorov | 2014-11-27 16:20:34 [+0600]:

Program will crash if nr_runs is 0 due to dividing by it in print_results(). Let's exit early instead.

Has this been dealt with? I can't see this applied. I would prefer doing the value check in parse_options() itself. And looking at it, it seems that run_interval is interval ignored. Or is there macro magic invovled?

Quoted 17 lines

---
src/rt-migrate-test/rt-migrate-test.c | 5 +++++

1 file changed, 5 insertions(+)
src/rt-migrate-test/rt-migrate-test.c +5 -0
index e3c7a09..876a122 100644
@@ -465,6 +465,11 @@ int main (int argc, char **argv)
 	parse_options(argc, argv);
+	if (nr_runs <= 0) {
+		fprintf(stderr, "Warning, --loops argument is non-positive. Exiting.\n");
+		exit(-1);
+	}
+
Quoted 13 lines
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines
src/rt-migrate-test/rt-migrate-test.c +0 -0
 	signal(SIGINT, stop_log);
 	if (argc >= (optind + 1))
Signature · 1 line
2.1.3

Sebastian

Signature · 3 lines
Best regards,
Boris Egorov
--

Awaiting your respin on these patches.

Thanks

John

#5 lore raw

v2 cover rt-migrate-test: few sanity fixes

In reply to Boris Egorov

Few fixes for rt-migrate-test, one to fix arithmetic error and others to properly handle options passed by user.

Difference of this series from the first patch:

* value checks now taking place in parse_options() instead of main()
* added second sanity patch

Boris Egorov (2):
rt-migrate-test: exit early if nr_runs is non-positive
rt-migrate-test: use variables instead of macros

 src/rt-migrate-test/rt-migrate-test.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
Signature · 1 line
2.1.4
#6 lore raw

PATCH v2 1/2 rt-migrate-test: exit early if nr_runs is non-positive

Program will crash if nr_runs is 0 due to dividing by it in print_results(). Let's exit early instead.

---
 src/rt-migrate-test/rt-migrate-test.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
src/rt-migrate-test/rt-migrate-test.c +0 -0
index e3c7a09..711d8a4 100644
@@ -226,7 +226,10 @@ static void parse_options (int argc, char *argv[])
 			break;
 		}
 	}
#7 lore raw

PATCH v2 2/2 rt-migrate-test: use variables instead of macros

In reply to Boris Egorov

By default, static variables run_interval and interval assigned to macros with corresponding uppercase name, RUN_INTERVAL and INTERVAL. Later in code we should only use these variables to properly handle options passed by user.

---
 src/rt-migrate-test/rt-migrate-test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
src/rt-migrate-test/rt-migrate-test.c +3 -3
index 711d8a4..cf682fe 100644
@@ -334,7 +334,7 @@ static unsigned long busy_loop(unsigned long long start_time)
 	do {
 		l++;
 		time = get_time();
-	} while ((time - start_time) < RUN_INTERVAL);
+	} while ((time - start_time) < run_interval);
 
 	return l;
 }
@@ -539,8 +539,8 @@ int main (int argc, char **argv)
 		
 
 
-	intv.tv_sec = nano2sec(INTERVAL);
-	intv.tv_nsec = INTERVAL % sec2nano(1);
+	intv.tv_sec = nano2sec(interval);
+	intv.tv_nsec = interval % sec2nano(1);
 
 	print_progress_bar(0);
 
Signature · 1 line
2.1.4
#8 lore raw

Re: [PATCH v2 0/2] rt-migrate-test: few sanity fixes

In reply to Boris Egorov
Quoted 8 lines · On Tue, 17 Feb 2015 23:43:57 +0600 Boris Egorov <[email protected]> wrote:

Few fixes for rt-migrate-test, one to fix arithmetic error and others to properly handle options passed by user.

Difference of this series from the first patch:

* value checks now taking place in parse_options() instead of main()
* added second sanity patch

Quoted 8 lines

Boris Egorov (2):
rt-migrate-test: exit early if nr_runs is non-positive
rt-migrate-test: use variables instead of macros

 src/rt-migrate-test/rt-migrate-test.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

I've pulled these into my tree and they will be in rt-tests-0.91. Should come out this week.

Clark