All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/15] cytune modernization
@ 2014-05-04 15:49 Sami Kerola
  2014-05-04 15:49 ` [PATCH 01/15] cytune: rename threshold and timeout variables Sami Kerola
                   ` (15 more replies)
  0 siblings, 16 replies; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

Hello all,

Couple days ago Benno Schulenberg mentioned email with subject 'cytune:
misnamed long options' usage() being a bit misleading that I concurred
with note that the cytune could probably be improved various ways.  This
patch set proposes the improvements I had in mind.

Please notice that I do not have hardware to test the cytune command, so
testing after the changes did not happen.  All I can say I tried to be
careful not to break program logic, and hopefully that will work.

p.s. The cytune and column changes are also available in the git
repository at:

  git://github.com/kerolasa/lelux-utiliteetit.git 2014wk17

Sami Kerola (15):
  cytune: rename threshold and timeout variables
  cytune: remove unnecessary variables
  cytune: be consistent with interval data type
  cytune: use single loop for setting and getting ioctl() calls
  cytune: kernel still does not have send_count in cyclades_monitor
    structure
  cytune: prefer sigaction(), and remove unnecessary abstractions
  cytune: add cyg_get_mon() to avoid duplicate code
  cytune: add structure to hold run time configuration
  cytune: pull signal handling and statistic priting apart
  cytune: remove unnecessary type casts
  cytune: deprecate undescriptive options
  cytune: add filename to struct cyclades_control
  cytune: add noreturn function attributes
  cytune: use matching type in struct cyclades_control with kernel
  cytune: update copyright

 sys-utils/cytune.8 |  10 +-
 sys-utils/cytune.c | 468 ++++++++++++++++++++++++-----------------------------
 2 files changed, 213 insertions(+), 265 deletions(-)

-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 01/15] cytune: rename threshold and timeout variables
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
@ 2014-05-04 15:49 ` Sami Kerola
  2014-05-05  8:34   ` Benno Schulenberg
  2014-05-04 15:49 ` [PATCH 02/15] cytune: remove unnecessary variables Sami Kerola
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

Especially the treshold was difficult to read, as the variable name had
no reference what it was setting.  Effect of the variables
s/get/get_current/ and s/get_def/get_defaults/ seem to documented wrong
way, that became apparent after looking into this name space.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/cytune.c | 101 +++++++++++++++++++++++++++--------------------------
 1 file changed, 52 insertions(+), 49 deletions(-)

diff --git a/sys-utils/cytune.c b/sys-utils/cytune.c
index 8a42f96..544865f 100644
--- a/sys-utils/cytune.c
+++ b/sys-utils/cytune.c
@@ -274,23 +274,26 @@ int main(int argc, char **argv)
 {
 	int query = 0;
 	int interval = 1;
-	int set = 0;
-	int set_val = -1;
-	int get = 0;
-	int set_def = 0;
-	int set_def_val = -1;
-	int get_def = 0;
-	int set_time = 0;
-	int set_time_val = -1;
-	int set_def_time = 0;
-	int set_def_time_val = -1;
 	int errflg = 0;
 	int file;
 	int numfiles;
 	int i;
+
+	int get_current = 0;
+	int get_defaults = 0;
 	unsigned long threshold_value;
 	unsigned long timeout_value;
 
+	int set_threshold = 0;
+	int threshold_val = -1;
+	int set_threshold_def = 0;
+	int threshold_def_val = -1;
+
+	int set_timeout = 0;
+	int timeout_val = -1;
+	int set_timeout_def = 0;
+	int timeout_def_val = -1;
+
 	static const struct option longopts[] = {
 		{"set-threshold", required_argument, NULL, 's'},
 		{"get-threshold", no_argument, NULL, 'g'},
@@ -331,48 +334,48 @@ int main(int argc, char **argv)
 			}
 			break;
 		case 's':
-			++set;
-			set_val = strtou32_or_err(optarg, _("Invalid set value"));
-			if (set_val < 1 || 12 < set_val) {
-				warnx(_("Invalid set value: %d"), set_val);
+			++set_threshold;
+			threshold_val = strtou32_or_err(optarg, _("Invalid threshold value"));
+			if (threshold_val < 1 || 12 < threshold_val) {
+				warnx(_("Invalid threshold value: %d"), threshold_val);
 				errflg++;
 			}
 			break;
 		case 'S':
-			++set_def;
-			set_def_val = strtou32_or_err(optarg,
-						_("Invalid default value"));
-			if (set_def_val < 0 || 12 < set_def_val) {
-				warnx(_("Invalid default value: %d"),
-				      set_def_val);
+			++set_threshold_def;
+			threshold_def_val = strtou32_or_err(optarg,
+						_("Invalid threshold default value"));
+			if (threshold_def_val < 0 || 12 < threshold_def_val) {
+				warnx(_("Invalid threshold default value: %d"),
+				      threshold_def_val);
 				errflg++;
 			}
 			break;
 		case 't':
-			++set_time;
-			set_time_val = strtou32_or_err(optarg,
-						_("Invalid set time value"));
-			if (set_time_val < 1 || 255 < set_time_val) {
-				warnx(_("Invalid set time value: %d"),
-				      set_time_val);
+			++set_timeout;
+			timeout_val = strtou32_or_err(optarg,
+						_("Invalid set timeout value"));
+			if (timeout_val < 1 || 255 < timeout_val) {
+				warnx(_("Invalid set timeout value: %d"),
+				      timeout_val);
 				errflg++;
 			}
 			break;
 		case 'T':
-			++set_def_time;
-			set_def_time_val = strtou32_or_err(optarg,
-						_("Invalid default time value"));
-			if (set_def_time_val < 0 || 255 < set_def_time_val) {
+			++set_threshold_def;
+			timeout_def_val = strtou32_or_err(optarg,
+						_("Invalid default timeout value"));
+			if (timeout_def_val < 0 || 255 < timeout_def_val) {
 				warnx(_("Invalid default time value: %d"),
-				      set_def_time_val);
+				      timeout_def_val);
 				errflg++;
 			}
 			break;
 		case 'g':
-			++get;
+			++get_current;
 			break;
 		case 'G':
-			++get_def;
+			++get_defaults;
 			break;
 		case 'V':
 			printf(_("%s from %s\n"), program_invocation_short_name,
@@ -388,62 +391,62 @@ int main(int argc, char **argv)
 
 	if (errflg
 	    || (numfiles == 0)
-	    || (!query && !set && !set_def && !get && !get_def && !set_time && !set_def_time)
-	    || (set && set_def)
-	    || (set_time && set_def_time)
-	    || (get && get_def))
+	    || (!query && !set_threshold && !set_threshold_def && !get_defaults && !get_current && !set_timeout && !set_timeout_def)
+	    || (set_threshold && set_threshold_def)
+	    || (set_timeout && set_timeout_def)
+	    || (get_defaults && get_current))
 		usage(stderr);
 
 	/* For signal routine. */
 	global_optind = optind;
 
-	if (set || set_def) {
+	if (set_threshold || set_threshold_def) {
 		for (i = optind; i < argc; i++) {
 			file = open(argv[i], O_RDONLY);
 			if (file == -1)
 				err(EXIT_FAILURE, _("cannot open %s"), argv[i]);
 			if (ioctl(file,
-				  set ? CYSETTHRESH : CYSETDEFTHRESH,
-				  set ? set_val : set_def_val))
+				  set_threshold ? CYSETTHRESH : CYSETDEFTHRESH,
+				  set_threshold ? threshold_val : threshold_def_val))
 				err(EXIT_FAILURE,
 				    _("cannot set %s to threshold %d"), argv[i],
-				    set ? set_val : set_def_val);
+				    set_threshold ? threshold_val : threshold_def_val);
 			close(file);
 		}
 	}
-	if (set_time || set_def_time) {
+	if (set_timeout || set_timeout_def) {
 		for (i = optind; i < argc; i++) {
 			file = open(argv[i], O_RDONLY);
 			if (file == -1)
 				err(EXIT_FAILURE, _("cannot open %s"), argv[i]);
 			if (ioctl(file,
-				  set_time ? CYSETTIMEOUT : CYSETDEFTIMEOUT,
-				  set_time ? set_time_val : set_def_time_val))
+				  set_timeout ? CYSETTIMEOUT : CYSETDEFTIMEOUT,
+				  set_timeout ? timeout_val : timeout_def_val))
 				err(EXIT_FAILURE,
 				    _("cannot set %s to time threshold %d"),
 				    argv[i],
-				    set_time ? set_time_val : set_def_time_val);
+				    set_timeout ? timeout_val : timeout_def_val);
 			close(file);
 		}
 	}
 
-	if (get || get_def) {
+	if (get_defaults || get_current) {
 		for (i = optind; i < argc; i++) {
 			file = open(argv[i], O_RDONLY);
 			if (file == -1)
 				err(EXIT_FAILURE, _("cannot open %s"), argv[i]);
 			if (ioctl
-			    (file, get ? CYGETTHRESH : CYGETDEFTHRESH,
+			    (file, get_defaults ? CYGETTHRESH : CYGETDEFTHRESH,
 			     &threshold_value))
 				err(EXIT_FAILURE,
 				    _("cannot get threshold for %s"), argv[i]);
 			if (ioctl
-			    (file, get ? CYGETTIMEOUT : CYGETDEFTIMEOUT,
+			    (file, get_current ? CYGETTIMEOUT : CYGETDEFTIMEOUT,
 			     &timeout_value))
 				err(EXIT_FAILURE,
 				    _("cannot get timeout for %s"), argv[i]);
 			close(file);
-			if (get)
+			if (get_defaults)
 				printf(_("%s: %ld current threshold and %ld current timeout\n"),
 					argv[i], threshold_value, timeout_value);
 			else
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 02/15] cytune: remove unnecessary variables
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
  2014-05-04 15:49 ` [PATCH 01/15] cytune: rename threshold and timeout variables Sami Kerola
@ 2014-05-04 15:49 ` Sami Kerola
  2014-05-05  8:29   ` Benno Schulenberg
  2014-05-04 15:49 ` [PATCH 03/15] cytune: be consistent with interval data type Sami Kerola
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

Exit immediately at error.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/cytune.c | 46 ++++++++++++++++------------------------------
 1 file changed, 16 insertions(+), 30 deletions(-)

diff --git a/sys-utils/cytune.c b/sys-utils/cytune.c
index 544865f..64e15cc 100644
--- a/sys-utils/cytune.c
+++ b/sys-utils/cytune.c
@@ -274,9 +274,7 @@ int main(int argc, char **argv)
 {
 	int query = 0;
 	int interval = 1;
-	int errflg = 0;
 	int file;
-	int numfiles;
 	int i;
 
 	int get_current = 0;
@@ -327,49 +325,39 @@ int main(int argc, char **argv)
 		case 'i':
 			interval = strtou32_or_err(optarg,
 						_("Invalid interval value"));
-			if (interval < 1) {
-				warnx(_("Invalid interval value: %d"),
-				      interval);
-				errflg++;
-			}
+			if (interval < 1)
+				errx(EXIT_FAILURE, _("Invalid interval value: %d"),
+						     interval);
 			break;
 		case 's':
 			++set_threshold;
 			threshold_val = strtou32_or_err(optarg, _("Invalid threshold value"));
-			if (threshold_val < 1 || 12 < threshold_val) {
-				warnx(_("Invalid threshold value: %d"), threshold_val);
-				errflg++;
-			}
+			if (threshold_val < 1 || 12 < threshold_val)
+				errx(EXIT_FAILURE, _("Invalid threshold value: %d"), threshold_val);
 			break;
 		case 'S':
 			++set_threshold_def;
 			threshold_def_val = strtou32_or_err(optarg,
 						_("Invalid threshold default value"));
-			if (threshold_def_val < 0 || 12 < threshold_def_val) {
-				warnx(_("Invalid threshold default value: %d"),
-				      threshold_def_val);
-				errflg++;
-			}
+			if (threshold_def_val < 0 || 12 < threshold_def_val)
+				errx(EXIT_FAILURE, ("Invalid threshold default value: %d"),
+						    threshold_def_val);
 			break;
 		case 't':
 			++set_timeout;
 			timeout_val = strtou32_or_err(optarg,
 						_("Invalid set timeout value"));
-			if (timeout_val < 1 || 255 < timeout_val) {
-				warnx(_("Invalid set timeout value: %d"),
-				      timeout_val);
-				errflg++;
-			}
+			if (timeout_val < 1 || 255 < timeout_val)
+				errx(EXIT_FAILURE, _("Invalid set timeout value: %d"),
+						     timeout_val);
 			break;
 		case 'T':
 			++set_threshold_def;
 			timeout_def_val = strtou32_or_err(optarg,
 						_("Invalid default timeout value"));
-			if (timeout_def_val < 0 || 255 < timeout_def_val) {
-				warnx(_("Invalid default time value: %d"),
-				      timeout_def_val);
-				errflg++;
-			}
+			if (timeout_def_val < 0 || 255 < timeout_def_val)
+				errx(EXIT_FAILURE, _("Invalid default time value: %d"),
+						     timeout_def_val);
 			break;
 		case 'g':
 			++get_current;
@@ -387,10 +375,8 @@ int main(int argc, char **argv)
 			usage(stderr);
 		}
 	}
-	numfiles = argc - optind;
 
-	if (errflg
-	    || (numfiles == 0)
+	if (argc - optind == 0
 	    || (!query && !set_threshold && !set_threshold_def && !get_defaults && !get_current && !set_timeout && !set_timeout_def)
 	    || (set_threshold && set_threshold_def)
 	    || (set_timeout && set_timeout_def)
@@ -458,7 +444,7 @@ int main(int argc, char **argv)
 	if (!query)
 		return EXIT_SUCCESS;
 
-	query_tty_stats(argc, argv, interval, numfiles, &threshold_value, &timeout_value);
+	query_tty_stats(argc, argv, interval, argc - optind, &threshold_value, &timeout_value);
 
 	return EXIT_SUCCESS;
 }
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 03/15] cytune: be consistent with interval data type
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
  2014-05-04 15:49 ` [PATCH 01/15] cytune: rename threshold and timeout variables Sami Kerola
  2014-05-04 15:49 ` [PATCH 02/15] cytune: remove unnecessary variables Sami Kerola
@ 2014-05-04 15:49 ` Sami Kerola
  2014-05-04 15:49 ` [PATCH 04/15] cytune: use single loop for setting and getting ioctl() calls Sami Kerola
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

The sleep(3) uses unsigned int as input.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/cytune.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sys-utils/cytune.c b/sys-utils/cytune.c
index 64e15cc..63f6980 100644
--- a/sys-utils/cytune.c
+++ b/sys-utils/cytune.c
@@ -149,7 +149,7 @@ static void summary(int sig)
 	cc->timeout_value = 0;
 }
 
-static void query_tty_stats(int argc, char **argv, int interval, int numfiles,
+static void query_tty_stats(int argc, char **argv, unsigned int interval, int numfiles,
 		     unsigned long *threshold_value,
 		     unsigned long *timeout_value)
 {
@@ -273,7 +273,7 @@ static void query_tty_stats(int argc, char **argv, int interval, int numfiles,
 int main(int argc, char **argv)
 {
 	int query = 0;
-	int interval = 1;
+	unsigned int interval = 1;
 	int file;
 	int i;
 
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 04/15] cytune: use single loop for setting and getting ioctl() calls
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
                   ` (2 preceding siblings ...)
  2014-05-04 15:49 ` [PATCH 03/15] cytune: be consistent with interval data type Sami Kerola
@ 2014-05-04 15:49 ` Sami Kerola
  2014-05-05  8:24   ` Benno Schulenberg
  2014-05-04 15:49 ` [PATCH 05/15] cytune: kernel still does not have send_count in cyclades_monitor structure Sami Kerola
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

This commit also fixes functional bug.  Documentation gives an idea one
might set current, and default values in one go but as a matter of fact
that did not work earlier.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/cytune.c | 93 +++++++++++++++++++++++-------------------------------
 1 file changed, 39 insertions(+), 54 deletions(-)

diff --git a/sys-utils/cytune.c b/sys-utils/cytune.c
index 63f6980..995a75f 100644
--- a/sys-utils/cytune.c
+++ b/sys-utils/cytune.c
@@ -386,65 +386,50 @@ int main(int argc, char **argv)
 	/* For signal routine. */
 	global_optind = optind;
 
-	if (set_threshold || set_threshold_def) {
-		for (i = optind; i < argc; i++) {
-			file = open(argv[i], O_RDONLY);
-			if (file == -1)
-				err(EXIT_FAILURE, _("cannot open %s"), argv[i]);
-			if (ioctl(file,
-				  set_threshold ? CYSETTHRESH : CYSETDEFTHRESH,
-				  set_threshold ? threshold_val : threshold_def_val))
-				err(EXIT_FAILURE,
-				    _("cannot set %s to threshold %d"), argv[i],
-				    set_threshold ? threshold_val : threshold_def_val);
-			close(file);
-		}
-	}
-	if (set_timeout || set_timeout_def) {
-		for (i = optind; i < argc; i++) {
-			file = open(argv[i], O_RDONLY);
-			if (file == -1)
-				err(EXIT_FAILURE, _("cannot open %s"), argv[i]);
-			if (ioctl(file,
-				  set_timeout ? CYSETTIMEOUT : CYSETDEFTIMEOUT,
-				  set_timeout ? timeout_val : timeout_def_val))
-				err(EXIT_FAILURE,
-				    _("cannot set %s to time threshold %d"),
-				    argv[i],
-				    set_timeout ? timeout_val : timeout_def_val);
-			close(file);
+	for (i = optind; i < argc; i++) {
+		file = open(argv[i], O_RDONLY);
+		if (file == -1)
+			err(EXIT_FAILURE, _("cannot open %s"), argv[i]);
+
+		if (set_threshold)
+			if (ioctl(file, CYSETTHRESH, threshold_val))
+				err(EXIT_FAILURE, _("cannot set %s to threshold %d"), argv[i], threshold_val);
+
+		if (set_threshold_def)
+			if (ioctl(file, CYSETDEFTHRESH, threshold_def_val))
+				err(EXIT_FAILURE, _("cannot set %s to threshold %d"), argv[i], threshold_def_val);
+
+		if (set_timeout)
+			if (ioctl(file, CYSETTIMEOUT, timeout_val))
+				err(EXIT_FAILURE, _("cannot set %s to time threshold %d"), argv[i], timeout_val);
+
+		if (set_timeout_def)
+			if (ioctl(file, CYSETDEFTIMEOUT, timeout_def_val))
+				err(EXIT_FAILURE, _("cannot set %s to time threshold %d"), argv[i], timeout_def_val);
+
+		if (get_defaults) {
+			if (ioctl(file, CYGETDEFTHRESH, &threshold_value))
+				err(EXIT_FAILURE, _("cannot get threshold for %s"), argv[i]);
+			if (ioctl(file, CYGETDEFTIMEOUT, &timeout_value))
+				err(EXIT_FAILURE, _("cannot get timeout for %s"), argv[i]);
+			printf(_("%s: %ld default threshold and %ld default timeout\n"),
+			       argv[i], threshold_value, timeout_value);
 		}
-	}
 
-	if (get_defaults || get_current) {
-		for (i = optind; i < argc; i++) {
-			file = open(argv[i], O_RDONLY);
-			if (file == -1)
-				err(EXIT_FAILURE, _("cannot open %s"), argv[i]);
-			if (ioctl
-			    (file, get_defaults ? CYGETTHRESH : CYGETDEFTHRESH,
-			     &threshold_value))
-				err(EXIT_FAILURE,
-				    _("cannot get threshold for %s"), argv[i]);
-			if (ioctl
-			    (file, get_current ? CYGETTIMEOUT : CYGETDEFTIMEOUT,
-			     &timeout_value))
-				err(EXIT_FAILURE,
-				    _("cannot get timeout for %s"), argv[i]);
-			close(file);
-			if (get_defaults)
-				printf(_("%s: %ld current threshold and %ld current timeout\n"),
-					argv[i], threshold_value, timeout_value);
-			else
-				printf(_("%s: %ld default threshold and %ld default timeout\n"),
-					argv[i], threshold_value, timeout_value);
+		if (get_current) {
+			if (ioctl(file, CYGETTHRESH, &threshold_value))
+				err(EXIT_FAILURE, _("cannot get threshold for %s"), argv[i]);
+			if (ioctl(file, CYGETTIMEOUT, &timeout_value))
+				err(EXIT_FAILURE, _("cannot get timeout for %s"), argv[i]);
+			printf(_("%s: %ld current threshold and %ld current timeout\n"),
+			       argv[i], threshold_value, timeout_value);
 		}
-	}
 
-	if (!query)
-		return EXIT_SUCCESS;
+		close(file);
+	}
 
-	query_tty_stats(argc, argv, interval, argc - optind, &threshold_value, &timeout_value);
+	if (query)
+		query_tty_stats(argc, argv, interval, argc - optind, &threshold_value, &timeout_value);
 
 	return EXIT_SUCCESS;
 }
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 05/15] cytune: kernel still does not have send_count in cyclades_monitor structure
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
                   ` (3 preceding siblings ...)
  2014-05-04 15:49 ` [PATCH 04/15] cytune: use single loop for setting and getting ioctl() calls Sami Kerola
@ 2014-05-04 15:49 ` Sami Kerola
  2014-05-04 15:49 ` [PATCH 06/15] cytune: prefer sigaction(), and remove unnecessary abstractions Sami Kerola
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

It is unlikely the information about sent data will ever appear to kernel
interface, so stop waiting it.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/cytune.c | 34 +---------------------------------
 1 file changed, 1 insertion(+), 33 deletions(-)

diff --git a/sys-utils/cytune.c b/sys-utils/cytune.c
index 995a75f..7ca39c7 100644
--- a/sys-utils/cytune.c
+++ b/sys-utils/cytune.c
@@ -55,26 +55,14 @@
 #include "closestream.h"
 #include "strutils.h"
 
-#if 0
-# ifndef XMIT
-#  include <linux/version.h>
-#  if LINUX_VERSION_CODE > 66056
-#   define XMIT
-#  endif
-# endif
-#endif
-
 #include "xalloc.h"
 #include "nls.h"
-/* Until it gets put in the kernel, toggle by hand. */
-#undef XMIT
 
 struct cyclades_control {
 	struct cyclades_monitor c;
 	int cfile;
 	int maxmax;
 	double maxtran;
-	double maxxmit;
 	unsigned long threshold_value;
 	unsigned long timeout_value;
 };
@@ -159,9 +147,6 @@ static void query_tty_stats(int argc, char **argv, unsigned int interval, int nu
 	int i;
 	double diff;
 	double xfer_rate;
-#ifdef XMIT
-	double xmit_rate;
-#endif
 
 	cmon = xmalloc(sizeof(struct cyclades_control) * numfiles);
 
@@ -214,9 +199,6 @@ static void query_tty_stats(int argc, char **argv, unsigned int interval, int nu
 				    _("cannot get timeout for %s"), argv[i]);
 
 			xfer_rate = cywork.char_count / diff;
-#ifdef XMIT
-			xmit_rate = cywork.send_count / diff;
-#endif
 			if ((*threshold_value) !=
 			    cmon[cmon_index].threshold_value
 			    || (*timeout_value) !=
@@ -232,10 +214,6 @@ static void query_tty_stats(int argc, char **argv, unsigned int interval, int nu
 				/* Don't record this first cycle after change */
 				if (xfer_rate > cmon[cmon_index].maxtran)
 					cmon[cmon_index].maxtran = xfer_rate;
-#ifdef XMIT
-				if (xmit_rate > cmon[cmon_index].maxxmit)
-					cmon[cmon_index].maxxmit = xmit_rate;
-#endif
 				if (cmon[cmon_index].maxmax < 0 ||
 				    cywork.char_max >
 				    (unsigned long)cmon[cmon_index].maxmax)
@@ -243,16 +221,6 @@ static void query_tty_stats(int argc, char **argv, unsigned int interval, int nu
 					    cywork.char_max;
 			}
 
-#ifdef XMIT
-			printf(_("%s: %lu ints, %lu/%lu chars; fifo: %lu thresh, %lu tmout, "
-				 "%lu max, %lu now\n"), argv[i],
-			       cywork.int_count, cywork.char_count,
-			       cywork.send_count, *threshold_value,
-			       *timeout_value, cywork.char_max,
-			       cywork.char_last);
-			printf(_("   %f int/sec; %f rec, %f send (char/sec)\n"),
-			       cywork.int_count / diff, xfer_rate, xmit_rate);
-#else
 			printf(_("%s: %lu ints, %lu chars; fifo: %lu thresh, %lu tmout, "
 				 "%lu max, %lu now\n"), argv[i],
 			       cywork.int_count, cywork.char_count,
@@ -260,7 +228,7 @@ static void query_tty_stats(int argc, char **argv, unsigned int interval, int nu
 			       cywork.char_last);
 			printf(_("   %f int/sec; %f rec (char/sec)\n"),
 			       cywork.int_count / diff, xfer_rate);
-#endif
+
 			memcpy(&cmon[cmon_index].c, &cywork,
 			       sizeof(struct cyclades_monitor));
 		}
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 06/15] cytune: prefer sigaction(), and remove unnecessary abstractions
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
                   ` (4 preceding siblings ...)
  2014-05-04 15:49 ` [PATCH 05/15] cytune: kernel still does not have send_count in cyclades_monitor structure Sami Kerola
@ 2014-05-04 15:49 ` Sami Kerola
  2014-05-04 15:49 ` [PATCH 07/15] cytune: add cyg_get_mon() to avoid duplicate code Sami Kerola
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

The signal(3) system call is not exactly deprecated, but it should not be
used in new code.  The gettimeofday() does not require timezone to be
real variable, so get rid of it.  The mvtime #define was in use only
which make literal expression more readable.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/cytune.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/sys-utils/cytune.c b/sys-utils/cytune.c
index 7ca39c7..4cc7db6 100644
--- a/sys-utils/cytune.c
+++ b/sys-utils/cytune.c
@@ -72,8 +72,6 @@ int cmon_index;
 static int global_argc, global_optind;
 static char ***global_argv;
 
-#define mvtime(tvpto, tvpfrom)  (((tvpto)->tv_sec = (tvpfrom)->tv_sec),(tvpto)->tv_usec = (tvpfrom)->tv_usec)
-
 static void __attribute__ ((__noreturn__)) usage(FILE * out)
 {
 	fprintf(out, USAGE_HEADER);
@@ -143,17 +141,22 @@ static void query_tty_stats(int argc, char **argv, unsigned int interval, int nu
 {
 	struct cyclades_monitor cywork;
 	struct timeval lasttime, thistime;
-	struct timezone tz = { 0, 0 };
 	int i;
 	double diff;
 	double xfer_rate;
+	struct sigaction sigact;
 
 	cmon = xmalloc(sizeof(struct cyclades_control) * numfiles);
 
-	if (signal(SIGINT, summary) ||
-	    signal(SIGQUIT, summary) || signal(SIGTERM, summary))
+	sigemptyset(&sigact.sa_mask);
+	sigact.sa_handler = &summary;
+	sigact.sa_flags = SA_RESTART;
+	if (sigaction(SIGINT, &sigact, NULL) ||
+	    sigaction(SIGQUIT, &sigact, NULL) ||
+	    sigaction(SIGTERM, &sigact, NULL))
 		err(EXIT_FAILURE, _("cannot set signal handler"));
-	if (gettimeofday(&lasttime, &tz))
+
+	if (gettimeofday(&lasttime, NULL))
 		err(EXIT_FAILURE, _("gettimeofday failed"));
 
 	for (i = optind; i < argc; i++) {
@@ -177,10 +180,11 @@ static void query_tty_stats(int argc, char **argv, unsigned int interval, int nu
 	while (1) {
 		sleep(interval);
 
-		if (gettimeofday(&thistime, &tz))
+		if (gettimeofday(&thistime, NULL))
 			err(EXIT_FAILURE, _("gettimeofday failed"));
 		diff = dtime(&thistime, &lasttime);
-		mvtime(&lasttime, &thistime);
+		lasttime.tv_sec = thistime.tv_sec;
+		lasttime.tv_usec = thistime.tv_usec;
 
 		for (i = optind; i < argc; i++) {
 			cmon_index = i - optind;
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 07/15] cytune: add cyg_get_mon() to avoid duplicate code
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
                   ` (5 preceding siblings ...)
  2014-05-04 15:49 ` [PATCH 06/15] cytune: prefer sigaction(), and remove unnecessary abstractions Sami Kerola
@ 2014-05-04 15:49 ` Sami Kerola
  2014-05-04 15:49 ` [PATCH 08/15] cytune: add structure to hold run time configuration Sami Kerola
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/cytune.c | 39 ++++++++++++++-------------------------
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/sys-utils/cytune.c b/sys-utils/cytune.c
index 4cc7db6..bdaad2e 100644
--- a/sys-utils/cytune.c
+++ b/sys-utils/cytune.c
@@ -135,6 +135,17 @@ static void summary(int sig)
 	cc->timeout_value = 0;
 }
 
+static void cyg_get_mon(int fd, struct cyclades_monitor *mon, char *file,
+			unsigned long *threshold_value, unsigned long *timeout_value)
+{
+	if (ioctl(fd, CYGETMON, &mon))
+		err(EXIT_FAILURE, _("cannot issue CYGETMON on %s"), file);
+	if (ioctl(fd, CYGETTHRESH, &threshold_value))
+		err(EXIT_FAILURE, _("cannot get threshold for %s"), file);
+	if (ioctl(fd, CYGETTIMEOUT, &timeout_value))
+		err(EXIT_FAILURE, _("cannot get timeout for %s"), file);
+}
+
 static void query_tty_stats(int argc, char **argv, unsigned int interval, int numfiles,
 		     unsigned long *threshold_value,
 		     unsigned long *timeout_value)
@@ -164,18 +175,8 @@ static void query_tty_stats(int argc, char **argv, unsigned int interval, int nu
 		cmon[cmon_index].cfile = open(argv[i], O_RDONLY);
 		if (cmon[cmon_index].cfile == -1)
 			err(EXIT_FAILURE, _("cannot open %s"), argv[i]);
-		if (ioctl
-		    (cmon[cmon_index].cfile, CYGETMON, &cmon[cmon_index].c))
-			err(EXIT_FAILURE, _("cannot issue CYGETMON on %s"),
-			    argv[i]);
+		cyg_get_mon(cmon[cmon_index].cfile, &cmon[cmon_index].c, argv[i], threshold_value, timeout_value);
 		summary(-1);
-		if (ioctl
-		    (cmon[cmon_index].cfile, CYGETTHRESH, &threshold_value))
-			err(EXIT_FAILURE, _("cannot get threshold for %s"),
-			    argv[i]);
-		if (ioctl(cmon[cmon_index].cfile, CYGETTIMEOUT, &timeout_value))
-			err(EXIT_FAILURE, _("cannot get timeout for %s"),
-			    argv[i]);
 	}
 	while (1) {
 		sleep(interval);
@@ -188,21 +189,9 @@ static void query_tty_stats(int argc, char **argv, unsigned int interval, int nu
 
 		for (i = optind; i < argc; i++) {
 			cmon_index = i - optind;
-			if (ioctl(cmon[cmon_index].cfile, CYGETMON, &cywork))
-				err(EXIT_FAILURE,
-				    _("cannot issue CYGETMON on %s"), argv[i]);
-			if (ioctl
-			    (cmon[cmon_index].cfile, CYGETTHRESH,
-			     &threshold_value))
-				err(EXIT_FAILURE,
-				    _("cannot get threshold for %s"), argv[i]);
-			if (ioctl
-			    (cmon[cmon_index].cfile, CYGETTIMEOUT,
-			     &timeout_value))
-				err(EXIT_FAILURE,
-				    _("cannot get timeout for %s"), argv[i]);
-
+			cyg_get_mon(cmon[cmon_index].cfile, &cywork, argv[i], threshold_value, timeout_value);
 			xfer_rate = cywork.char_count / diff;
+
 			if ((*threshold_value) !=
 			    cmon[cmon_index].threshold_value
 			    || (*timeout_value) !=
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 08/15] cytune: add structure to hold run time configuration
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
                   ` (6 preceding siblings ...)
  2014-05-04 15:49 ` [PATCH 07/15] cytune: add cyg_get_mon() to avoid duplicate code Sami Kerola
@ 2014-05-04 15:49 ` Sami Kerola
  2014-05-04 15:49 ` [PATCH 09/15] cytune: pull signal handling and statistic priting apart Sami Kerola
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/cytune.c | 220 +++++++++++++++++++++++++++--------------------------
 1 file changed, 113 insertions(+), 107 deletions(-)

diff --git a/sys-utils/cytune.c b/sys-utils/cytune.c
index bdaad2e..8dd566d 100644
--- a/sys-utils/cytune.c
+++ b/sys-utils/cytune.c
@@ -67,10 +67,29 @@ struct cyclades_control {
 	unsigned long timeout_value;
 };
 struct cyclades_control *cmon;
-int cmon_index;
 
-static int global_argc, global_optind;
-static char ***global_argv;
+struct cytune_modifiers {
+	int global_argc;
+	int global_optind;
+	char ***global_argv;
+	int cmon_index;
+	unsigned int interval;
+	unsigned long threshold_value;
+	unsigned long timeout_value;
+	int threshold_val;
+	int threshold_def_val;
+	int timeout_val;
+	int timeout_def_val;
+	unsigned int
+	    query:1,
+	    get_current:1,
+	    get_defaults:1,
+	    set_threshold:1,
+	    set_threshold_def:1,
+	    set_timeout:1,
+	    set_timeout_def:1;
+};
+struct cytune_modifiers mod;
 
 static void __attribute__ ((__noreturn__)) usage(FILE * out)
 {
@@ -108,13 +127,13 @@ static void summary(int sig)
 	char **argv;
 	int i;
 
-	argc = global_argc;
-	argv = *global_argv;
-	local_optind = global_optind;
+	argc = mod.global_argc;
+	argv = *mod.global_argv;
+	local_optind = mod.global_optind;
 
 	if (sig > 0) {
 		for (i = local_optind; i < argc; i++) {
-			cc = &cmon[cmon_index];
+			cc = &cmon[mod.cmon_index];
 			warnx(_("File %s, For threshold value %lu, Maximum characters in fifo were %d,\n"
 				"and the maximum transfer rate in characters/second was %f"),
 			      argv[i], cc->threshold_value, cc->maxmax,
@@ -122,11 +141,11 @@ static void summary(int sig)
 		}
 		exit(EXIT_SUCCESS);
 	}
-	cc = &cmon[cmon_index];
+	cc = &cmon[mod.cmon_index];
 	if (cc->threshold_value > 0 && sig != -1) {
 		warnx(_("File %s, For threshold value %lu and timeout value %lu, Maximum characters in fifo were %d,\n"
 			"and the maximum transfer rate in characters/second was %f"),
-		      argv[cmon_index + local_optind], cc->threshold_value,
+		      argv[mod.cmon_index + local_optind], cc->threshold_value,
 		      cc->timeout_value, cc->maxmax, cc->maxtran);
 	}
 	cc->maxmax = 0;
@@ -136,19 +155,17 @@ static void summary(int sig)
 }
 
 static void cyg_get_mon(int fd, struct cyclades_monitor *mon, char *file,
-			unsigned long *threshold_value, unsigned long *timeout_value)
+			struct cytune_modifiers *mod)
 {
 	if (ioctl(fd, CYGETMON, &mon))
 		err(EXIT_FAILURE, _("cannot issue CYGETMON on %s"), file);
-	if (ioctl(fd, CYGETTHRESH, &threshold_value))
+	if (ioctl(fd, CYGETTHRESH, &mod->threshold_value))
 		err(EXIT_FAILURE, _("cannot get threshold for %s"), file);
-	if (ioctl(fd, CYGETTIMEOUT, &timeout_value))
+	if (ioctl(fd, CYGETTIMEOUT, &mod->timeout_value))
 		err(EXIT_FAILURE, _("cannot get timeout for %s"), file);
 }
 
-static void query_tty_stats(int argc, char **argv, unsigned int interval, int numfiles,
-		     unsigned long *threshold_value,
-		     unsigned long *timeout_value)
+static void query_tty_stats(int argc, char **argv, struct cytune_modifiers *mod)
 {
 	struct cyclades_monitor cywork;
 	struct timeval lasttime, thistime;
@@ -157,7 +174,7 @@ static void query_tty_stats(int argc, char **argv, unsigned int interval, int nu
 	double xfer_rate;
 	struct sigaction sigact;
 
-	cmon = xmalloc(sizeof(struct cyclades_control) * numfiles);
+	cmon = xmalloc(sizeof(struct cyclades_control) * (argc - mod->global_optind));
 
 	sigemptyset(&sigact.sa_mask);
 	sigact.sa_handler = &summary;
@@ -171,15 +188,15 @@ static void query_tty_stats(int argc, char **argv, unsigned int interval, int nu
 		err(EXIT_FAILURE, _("gettimeofday failed"));
 
 	for (i = optind; i < argc; i++) {
-		cmon_index = i - optind;
-		cmon[cmon_index].cfile = open(argv[i], O_RDONLY);
-		if (cmon[cmon_index].cfile == -1)
+		mod->cmon_index = i - optind;
+		cmon[mod->cmon_index].cfile = open(argv[i], O_RDONLY);
+		if (cmon[mod->cmon_index].cfile == -1)
 			err(EXIT_FAILURE, _("cannot open %s"), argv[i]);
-		cyg_get_mon(cmon[cmon_index].cfile, &cmon[cmon_index].c, argv[i], threshold_value, timeout_value);
+		cyg_get_mon(cmon[mod->cmon_index].cfile, &cmon[mod->cmon_index].c, argv[i], mod);
 		summary(-1);
 	}
 	while (1) {
-		sleep(interval);
+		sleep(mod->interval);
 
 		if (gettimeofday(&thistime, NULL))
 			err(EXIT_FAILURE, _("gettimeofday failed"));
@@ -188,41 +205,41 @@ static void query_tty_stats(int argc, char **argv, unsigned int interval, int nu
 		lasttime.tv_usec = thistime.tv_usec;
 
 		for (i = optind; i < argc; i++) {
-			cmon_index = i - optind;
-			cyg_get_mon(cmon[cmon_index].cfile, &cywork, argv[i], threshold_value, timeout_value);
+			mod->cmon_index = i - optind;
+			cyg_get_mon(cmon[mod->cmon_index].cfile, &cywork, argv[i], mod);
 			xfer_rate = cywork.char_count / diff;
 
-			if ((*threshold_value) !=
-			    cmon[cmon_index].threshold_value
-			    || (*timeout_value) !=
-			    cmon[cmon_index].timeout_value) {
+			if (mod->threshold_value !=
+			    cmon[mod->cmon_index].threshold_value
+			    || mod->timeout_value !=
+			    cmon[mod->cmon_index].timeout_value) {
 				summary(-2);
 				/* Note that the summary must come before the
 				 * setting of threshold_value */
-				cmon[cmon_index].threshold_value =
-				    (*threshold_value);
-				cmon[cmon_index].timeout_value =
-				    (*timeout_value);
+				cmon[mod->cmon_index].threshold_value =
+				    mod->threshold_value;
+				cmon[mod->cmon_index].timeout_value =
+				    mod->timeout_value;
 			} else {
 				/* Don't record this first cycle after change */
-				if (xfer_rate > cmon[cmon_index].maxtran)
-					cmon[cmon_index].maxtran = xfer_rate;
-				if (cmon[cmon_index].maxmax < 0 ||
+				if (xfer_rate > cmon[mod->cmon_index].maxtran)
+					cmon[mod->cmon_index].maxtran = xfer_rate;
+				if (cmon[mod->cmon_index].maxmax < 0 ||
 				    cywork.char_max >
-				    (unsigned long)cmon[cmon_index].maxmax)
-					cmon[cmon_index].maxmax =
+				    (unsigned long)cmon[mod->cmon_index].maxmax)
+					cmon[mod->cmon_index].maxmax =
 					    cywork.char_max;
 			}
 
 			printf(_("%s: %lu ints, %lu chars; fifo: %lu thresh, %lu tmout, "
 				 "%lu max, %lu now\n"), argv[i],
 			       cywork.int_count, cywork.char_count,
-			       *threshold_value, *timeout_value, cywork.char_max,
+			       mod->threshold_value, mod->timeout_value, cywork.char_max,
 			       cywork.char_last);
 			printf(_("   %f int/sec; %f rec (char/sec)\n"),
 			       cywork.int_count / diff, xfer_rate);
 
-			memcpy(&cmon[cmon_index].c, &cywork,
+			memcpy(&cmon[mod->cmon_index].c, &cywork,
 			       sizeof(struct cyclades_monitor));
 		}
 	}
@@ -233,25 +250,10 @@ static void query_tty_stats(int argc, char **argv, unsigned int interval, int nu
 
 int main(int argc, char **argv)
 {
-	int query = 0;
-	unsigned int interval = 1;
 	int file;
 	int i;
-
-	int get_current = 0;
-	int get_defaults = 0;
-	unsigned long threshold_value;
-	unsigned long timeout_value;
-
-	int set_threshold = 0;
-	int threshold_val = -1;
-	int set_threshold_def = 0;
-	int threshold_def_val = -1;
-
-	int set_timeout = 0;
-	int timeout_val = -1;
-	int set_timeout_def = 0;
-	int timeout_def_val = -1;
+	int intention_expressed = 0;
+	struct cytune_modifiers mod;
 
 	static const struct option longopts[] = {
 		{"set-threshold", required_argument, NULL, 's'},
@@ -268,63 +270,71 @@ int main(int argc, char **argv)
 	};
 
 	/* For signal routine. */
-	global_argc = argc;
-	global_argv = &argv;
+	mod.global_argc = argc;
+	mod.global_argv = &argv;
 
 	setlocale(LC_ALL, "");
 	bindtextdomain(PACKAGE, LOCALEDIR);
 	textdomain(PACKAGE);
 	atexit(close_stdout);
+	mod.interval = 1;
 
 	while ((i =
 		getopt_long(argc, argv, "qs:S:t:T:gGi:Vh", longopts,
 			    NULL)) != -1) {
 		switch (i) {
 		case 'q':
-			query = 1;
+			mod.query = 1;
+			intention_expressed = 1;
 			break;
 		case 'i':
-			interval = strtou32_or_err(optarg,
+			mod.interval = strtou32_or_err(optarg,
 						_("Invalid interval value"));
-			if (interval < 1)
+			if (mod.interval < 1)
 				errx(EXIT_FAILURE, _("Invalid interval value: %d"),
-						     interval);
+						     mod.interval);
 			break;
 		case 's':
-			++set_threshold;
-			threshold_val = strtou32_or_err(optarg, _("Invalid threshold value"));
-			if (threshold_val < 1 || 12 < threshold_val)
-				errx(EXIT_FAILURE, _("Invalid threshold value: %d"), threshold_val);
+			mod.set_threshold = 1;
+			intention_expressed = 1;
+			mod.threshold_val = strtou32_or_err(optarg, _("Invalid threshold value"));
+			if (mod.threshold_val < 1 || 12 < mod.threshold_val)
+				errx(EXIT_FAILURE, _("Invalid threshold value: %d"), mod.threshold_val);
 			break;
 		case 'S':
-			++set_threshold_def;
-			threshold_def_val = strtou32_or_err(optarg,
+			mod.set_threshold_def = 1;
+			intention_expressed = 1;
+			mod.threshold_def_val = strtou32_or_err(optarg,
 						_("Invalid threshold default value"));
-			if (threshold_def_val < 0 || 12 < threshold_def_val)
+			if (mod.threshold_def_val < 0 || 12 < mod.threshold_def_val)
 				errx(EXIT_FAILURE, ("Invalid threshold default value: %d"),
-						    threshold_def_val);
+						    mod.threshold_def_val);
 			break;
 		case 't':
-			++set_timeout;
-			timeout_val = strtou32_or_err(optarg,
+			mod.set_timeout = 1;
+			intention_expressed = 1;
+			mod.timeout_val = strtou32_or_err(optarg,
 						_("Invalid set timeout value"));
-			if (timeout_val < 1 || 255 < timeout_val)
+			if (mod.timeout_val < 1 || 255 < mod.timeout_val)
 				errx(EXIT_FAILURE, _("Invalid set timeout value: %d"),
-						     timeout_val);
+						     mod.timeout_val);
 			break;
 		case 'T':
-			++set_threshold_def;
-			timeout_def_val = strtou32_or_err(optarg,
+			mod.set_threshold_def = 1;
+			intention_expressed = 1;
+			mod.timeout_def_val = strtou32_or_err(optarg,
 						_("Invalid default timeout value"));
-			if (timeout_def_val < 0 || 255 < timeout_def_val)
+			if (mod.timeout_def_val < 0 || 255 < mod.timeout_def_val)
 				errx(EXIT_FAILURE, _("Invalid default time value: %d"),
-						     timeout_def_val);
+						     mod.timeout_def_val);
 			break;
 		case 'g':
-			++get_current;
+			mod.get_current = 1;
+			intention_expressed = 1;
 			break;
 		case 'G':
-			++get_defaults;
+			mod.get_defaults = 1;
+			intention_expressed = 1;
 			break;
 		case 'V':
 			printf(_("%s from %s\n"), program_invocation_short_name,
@@ -337,60 +347,56 @@ int main(int argc, char **argv)
 		}
 	}
 
-	if (argc - optind == 0
-	    || (!query && !set_threshold && !set_threshold_def && !get_defaults && !get_current && !set_timeout && !set_timeout_def)
-	    || (set_threshold && set_threshold_def)
-	    || (set_timeout && set_timeout_def)
-	    || (get_defaults && get_current))
+	if (argc - optind == 0 || !intention_expressed)
 		usage(stderr);
 
 	/* For signal routine. */
-	global_optind = optind;
+	mod.global_optind = optind;
 
 	for (i = optind; i < argc; i++) {
 		file = open(argv[i], O_RDONLY);
 		if (file == -1)
 			err(EXIT_FAILURE, _("cannot open %s"), argv[i]);
 
-		if (set_threshold)
-			if (ioctl(file, CYSETTHRESH, threshold_val))
-				err(EXIT_FAILURE, _("cannot set %s to threshold %d"), argv[i], threshold_val);
+		if (mod.set_threshold)
+			if (ioctl(file, CYSETTHRESH, mod.threshold_val))
+				err(EXIT_FAILURE, _("cannot set %s to threshold %d"), argv[i], mod.threshold_val);
 
-		if (set_threshold_def)
-			if (ioctl(file, CYSETDEFTHRESH, threshold_def_val))
-				err(EXIT_FAILURE, _("cannot set %s to threshold %d"), argv[i], threshold_def_val);
+		if (mod.set_threshold_def)
+			if (ioctl(file, CYSETDEFTHRESH, mod.threshold_def_val))
+				err(EXIT_FAILURE, _("cannot set %s to threshold %d"), argv[i], mod.threshold_def_val);
 
-		if (set_timeout)
-			if (ioctl(file, CYSETTIMEOUT, timeout_val))
-				err(EXIT_FAILURE, _("cannot set %s to time threshold %d"), argv[i], timeout_val);
+		if (mod.set_timeout)
+			if (ioctl(file, CYSETTIMEOUT, mod.timeout_val))
+				err(EXIT_FAILURE, _("cannot set %s to time threshold %d"), argv[i], mod.timeout_val);
 
-		if (set_timeout_def)
-			if (ioctl(file, CYSETDEFTIMEOUT, timeout_def_val))
-				err(EXIT_FAILURE, _("cannot set %s to time threshold %d"), argv[i], timeout_def_val);
+		if (mod.set_timeout_def)
+			if (ioctl(file, CYSETDEFTIMEOUT, mod.timeout_def_val))
+				err(EXIT_FAILURE, _("cannot set %s to time threshold %d"), argv[i], mod.timeout_def_val);
 
-		if (get_defaults) {
-			if (ioctl(file, CYGETDEFTHRESH, &threshold_value))
+		if (mod.get_defaults) {
+			if (ioctl(file, CYGETDEFTHRESH, &mod.threshold_value))
 				err(EXIT_FAILURE, _("cannot get threshold for %s"), argv[i]);
-			if (ioctl(file, CYGETDEFTIMEOUT, &timeout_value))
+			if (ioctl(file, CYGETDEFTIMEOUT, &mod.timeout_value))
 				err(EXIT_FAILURE, _("cannot get timeout for %s"), argv[i]);
 			printf(_("%s: %ld default threshold and %ld default timeout\n"),
-			       argv[i], threshold_value, timeout_value);
+			       argv[i], mod.threshold_value, mod.timeout_value);
 		}
 
-		if (get_current) {
-			if (ioctl(file, CYGETTHRESH, &threshold_value))
+		if (mod.get_current) {
+			if (ioctl(file, CYGETTHRESH, &mod.threshold_value))
 				err(EXIT_FAILURE, _("cannot get threshold for %s"), argv[i]);
-			if (ioctl(file, CYGETTIMEOUT, &timeout_value))
+			if (ioctl(file, CYGETTIMEOUT, &mod.timeout_value))
 				err(EXIT_FAILURE, _("cannot get timeout for %s"), argv[i]);
 			printf(_("%s: %ld current threshold and %ld current timeout\n"),
-			       argv[i], threshold_value, timeout_value);
+			       argv[i], mod.threshold_value, mod.timeout_value);
 		}
 
 		close(file);
 	}
 
-	if (query)
-		query_tty_stats(argc, argv, interval, argc - optind, &threshold_value, &timeout_value);
+	if (mod.query)
+		query_tty_stats(argc, argv, &mod);
 
 	return EXIT_SUCCESS;
 }
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 09/15] cytune: pull signal handling and statistic priting apart
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
                   ` (7 preceding siblings ...)
  2014-05-04 15:49 ` [PATCH 08/15] cytune: add structure to hold run time configuration Sami Kerola
@ 2014-05-04 15:49 ` Sami Kerola
  2014-05-04 15:49 ` [PATCH 10/15] cytune: remove unnecessary type casts Sami Kerola
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

Combination of these two functions made the code to rely to global
variables, and cmon_index was seemingly invalid if signal was sent at a
time when query_tty_stats() was half done with a statistics printout.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/cytune.c | 60 +++++++++++++++++++++++++-----------------------------
 1 file changed, 28 insertions(+), 32 deletions(-)

diff --git a/sys-utils/cytune.c b/sys-utils/cytune.c
index 8dd566d..4f0ddc7 100644
--- a/sys-utils/cytune.c
+++ b/sys-utils/cytune.c
@@ -120,33 +120,36 @@ static inline double dtime(struct timeval *tvpnew, struct timeval *tvpold)
 	return diff;
 }
 
-static void summary(int sig)
+static void signal_handler(int sig __attribute__((__unused__)))
 {
 	struct cyclades_control *cc;
 	int argc, local_optind;
 	char **argv;
-	int i;
+	int i, j = 0;
 
 	argc = mod.global_argc;
 	argv = *mod.global_argv;
 	local_optind = mod.global_optind;
 
-	if (sig > 0) {
-		for (i = local_optind; i < argc; i++) {
-			cc = &cmon[mod.cmon_index];
-			warnx(_("File %s, For threshold value %lu, Maximum characters in fifo were %d,\n"
-				"and the maximum transfer rate in characters/second was %f"),
-			      argv[i], cc->threshold_value, cc->maxmax,
-			      cc->maxtran);
-		}
-		exit(EXIT_SUCCESS);
+	for (i = local_optind; i < argc; i++, j++) {
+		cc = &cmon[j];
+		warnx(_("File %s, For threshold value %lu, Maximum characters in fifo were %d,\n"
+			"and the maximum transfer rate in characters/second was %f"),
+		      argv[i], cc->threshold_value, cc->maxmax, cc->maxtran);
 	}
-	cc = &cmon[mod.cmon_index];
-	if (cc->threshold_value > 0 && sig != -1) {
+	exit(EXIT_SUCCESS);
+}
+
+static void print_statistics(struct cyclades_control *mon, struct cytune_modifiers *mod, int init)
+{
+	struct cyclades_control *cc;
+
+	cc = &mon[mod->cmon_index];
+	if (0 < cc->threshold_value && !init) {
 		warnx(_("File %s, For threshold value %lu and timeout value %lu, Maximum characters in fifo were %d,\n"
 			"and the maximum transfer rate in characters/second was %f"),
-		      argv[mod.cmon_index + local_optind], cc->threshold_value,
-		      cc->timeout_value, cc->maxmax, cc->maxtran);
+		      *mod->global_argv[mod->cmon_index + optind], cc->threshold_value, cc->timeout_value, cc->maxmax,
+		      cc->maxtran);
 	}
 	cc->maxmax = 0;
 	cc->maxtran = 0.0;
@@ -177,7 +180,7 @@ static void query_tty_stats(int argc, char **argv, struct cytune_modifiers *mod)
 	cmon = xmalloc(sizeof(struct cyclades_control) * (argc - mod->global_optind));
 
 	sigemptyset(&sigact.sa_mask);
-	sigact.sa_handler = &summary;
+	sigact.sa_handler = &signal_handler;
 	sigact.sa_flags = SA_RESTART;
 	if (sigaction(SIGINT, &sigact, NULL) ||
 	    sigaction(SIGQUIT, &sigact, NULL) ||
@@ -193,7 +196,7 @@ static void query_tty_stats(int argc, char **argv, struct cytune_modifiers *mod)
 		if (cmon[mod->cmon_index].cfile == -1)
 			err(EXIT_FAILURE, _("cannot open %s"), argv[i]);
 		cyg_get_mon(cmon[mod->cmon_index].cfile, &cmon[mod->cmon_index].c, argv[i], mod);
-		summary(-1);
+		print_statistics(&cmon[mod->cmon_index], mod, 1);
 	}
 	while (1) {
 		sleep(mod->interval);
@@ -209,28 +212,21 @@ static void query_tty_stats(int argc, char **argv, struct cytune_modifiers *mod)
 			cyg_get_mon(cmon[mod->cmon_index].cfile, &cywork, argv[i], mod);
 			xfer_rate = cywork.char_count / diff;
 
-			if (mod->threshold_value !=
-			    cmon[mod->cmon_index].threshold_value
-			    || mod->timeout_value !=
-			    cmon[mod->cmon_index].timeout_value) {
-				summary(-2);
+			if (mod->threshold_value != cmon[mod->cmon_index].threshold_value ||
+			    mod->timeout_value != cmon[mod->cmon_index].timeout_value) {
+				print_statistics(&cmon[mod->cmon_index], mod, 0);
 				/* Note that the summary must come before the
 				 * setting of threshold_value */
-				cmon[mod->cmon_index].threshold_value =
-				    mod->threshold_value;
-				cmon[mod->cmon_index].timeout_value =
-				    mod->timeout_value;
+				cmon[mod->cmon_index].threshold_value = mod->threshold_value;
+				cmon[mod->cmon_index].timeout_value = mod->timeout_value;
 			} else {
 				/* Don't record this first cycle after change */
-				if (xfer_rate > cmon[mod->cmon_index].maxtran)
+				if (cmon[mod->cmon_index].maxtran < xfer_rate)
 					cmon[mod->cmon_index].maxtran = xfer_rate;
 				if (cmon[mod->cmon_index].maxmax < 0 ||
-				    cywork.char_max >
-				    (unsigned long)cmon[mod->cmon_index].maxmax)
-					cmon[mod->cmon_index].maxmax =
-					    cywork.char_max;
+				    (unsigned long)cmon[mod->cmon_index].maxmax < cywork.char_max)
+					cmon[mod->cmon_index].maxmax = cywork.char_max;
 			}
-
 			printf(_("%s: %lu ints, %lu chars; fifo: %lu thresh, %lu tmout, "
 				 "%lu max, %lu now\n"), argv[i],
 			       cywork.int_count, cywork.char_count,
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 10/15] cytune: remove unnecessary type casts
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
                   ` (8 preceding siblings ...)
  2014-05-04 15:49 ` [PATCH 09/15] cytune: pull signal handling and statistic priting apart Sami Kerola
@ 2014-05-04 15:49 ` Sami Kerola
  2014-05-04 15:49 ` [PATCH 11/15] cytune: deprecate undescriptive options Sami Kerola
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/cytune.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sys-utils/cytune.c b/sys-utils/cytune.c
index 4f0ddc7..5bb1328 100644
--- a/sys-utils/cytune.c
+++ b/sys-utils/cytune.c
@@ -115,8 +115,8 @@ static void __attribute__ ((__noreturn__)) usage(FILE * out)
 static inline double dtime(struct timeval *tvpnew, struct timeval *tvpold)
 {
 	double diff;
-	diff = (double)tvpnew->tv_sec - (double)tvpold->tv_sec;
-	diff += ((double)tvpnew->tv_usec - (double)tvpold->tv_usec) / 1000000;
+	diff = (double)(tvpnew->tv_sec - tvpold->tv_sec);
+	diff += (double)(tvpnew->tv_usec - tvpold->tv_usec) / 1000000;
 	return diff;
 }
 
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 11/15] cytune: deprecate undescriptive options
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
                   ` (9 preceding siblings ...)
  2014-05-04 15:49 ` [PATCH 10/15] cytune: remove unnecessary type casts Sami Kerola
@ 2014-05-04 15:49 ` Sami Kerola
  2014-05-04 15:49 ` [PATCH 12/15] cytune: add filename to struct cyclades_control Sami Kerola
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

The options with 'flush' in the name are not very good.  Use of 'timeout'
instead is much more appropriate.

Reviewed-by: Benno Schulenberg <bensberg@justemail.net>
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/cytune.8 | 10 +++++-----
 sys-utils/cytune.c | 39 ++++++++++++++++++++++++++++-----------
 2 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/sys-utils/cytune.8 b/sys-utils/cytune.8
index 940fa28..4f4daa9 100644
--- a/sys-utils/cytune.8
+++ b/sys-utils/cytune.8
@@ -24,7 +24,7 @@
 .\" Formatted or processed versions of this manual, if unaccompanied by
 .\" the source, must acknowledge the copyright and authors of this work.
 .\" "
-.TH CYTUNE 8 "September 2011" "util-linux" "System Administration"
+.TH CYTUNE 8 "May 2014" "util-linux" "System Administration"
 .SH NAME
 cytune \- tune driver parameters for Cyclades-Z multiport serial card
 .SH SYNOPSIS
@@ -125,7 +125,7 @@ characters.  Note that if the
 is not being held open by another process, the threshold will be reset on the
 next open.  Only values between 1 and 12, inclusive, are permitted.
 .TP
-\fB\-t\fR, \fB\-\-set\-flush\fR \fIvalue\fR
+\fB\-t\fR, \fB\-\-set\-timeout\fR \fIvalue\fR
 Set the current flush timeout to
 .I value
 units.  Note that if the
@@ -136,7 +136,7 @@ next open.  Only values between 0 and 255, inclusive, are permitted.  Setting
 to zero forces the default, currently 0x20 (160ms), but soon to be 0x02
 (10ms).  Units are 5 ms.
 .TP
-\fB\-g\fR, \fB\-\-get\-threshold\fR
+\fB\-g\fR, \fB\-\-get\-current\fR
 Get the current threshold and timeout.
 .TP
 \fB\-S\fR, \fB\-\-set\-default\-threshold\fR \fIvalue\fR
@@ -147,7 +147,7 @@ characters.  When the
 is next opened, this value will be used instead of the default.  Only values
 between 1 and 12, inclusive, are permitted.
 .TP
-\fB\-T\fR, \fB\-\-set\-default\-flush\fR \fIvalue\fR
+\fB\-T\fR, \fB\-\-set\-default\-timeout\fR \fIvalue\fR
 Set the default flush timeout to
 .I value
 units.  When the
@@ -157,7 +157,7 @@ is next opened, this value will be used instead of the default.  If
 is zero, then the value will default to 0x20 (160ms), soon to be 0x02
 (10ms).
 .TP
-\fB\-G\fR, \fB\-\-get\-glush\fR
+\fB\-G\fR, \fB\-\-get\-defaults\fR
 Get the default threshold and flush timeout values.
 .TP
 \fB\-q\fR, \fB\-\-stats\fR
diff --git a/sys-utils/cytune.c b/sys-utils/cytune.c
index 5bb1328..0f97e28 100644
--- a/sys-utils/cytune.c
+++ b/sys-utils/cytune.c
@@ -96,12 +96,12 @@ static void __attribute__ ((__noreturn__)) usage(FILE * out)
 	fprintf(out, USAGE_HEADER);
 	fprintf(out, _(" %s [options] <tty> [...]\n"), program_invocation_short_name);
 	fprintf(out, USAGE_OPTIONS);
+	fprintf(out, _(" -g, --get-current                  display current values\n"));
+	fprintf(out, _(" -G, --get-defaults                 display default values\n"));
 	fprintf(out, _(" -s, --set-threshold <num>          set interruption threshold value\n"));
-	fprintf(out, _(" -g, --get-threshold                display current threshold value\n"));
 	fprintf(out, _(" -S, --set-default-threshold <num>  set default threshold value\n"));
-	fprintf(out, _(" -t, --set-flush <num>              set flush timeout to value\n"));
-	fprintf(out, _(" -G, --get-flush                    display default flush timeout value\n"));
-	fprintf(out, _(" -T, --set-default-flush <num>      set the default flush timeout to value\n"));
+	fprintf(out, _(" -t, --set-timeout <num>            set flush timeout to value\n"));
+	fprintf(out, _(" -T, --set-default-timeout <num>    set the default flush timeout to value\n"));
 	fprintf(out, _(" -q, --stats                        display statistics about the tty\n"));
 	fprintf(out, _(" -i, --interval <seconds>           gather statistics every <seconds> interval\n"));
 	fprintf(out, USAGE_SEPARATOR);
@@ -251,13 +251,23 @@ int main(int argc, char **argv)
 	int intention_expressed = 0;
 	struct cytune_modifiers mod;
 
+	enum {	/* FIXME: remove after May 2016. */
+		OPT_GET_TRESHHOLD = CHAR_MAX + 1,
+		OPT_GET_FLUSH,
+		OPT_SET_FLUSH,
+		OPT_SET_FLUSH_DEFAULT
+	};
 	static const struct option longopts[] = {
+		{"get-threshold", no_argument, NULL, OPT_GET_TRESHHOLD},
+		{"get-flush", no_argument, NULL, OPT_GET_FLUSH},
+		{"set-flush", required_argument, NULL, OPT_SET_FLUSH},
+		{"set-default-flush", required_argument, NULL, OPT_SET_FLUSH_DEFAULT},
+		{"get-current", no_argument, NULL, 'g'},
+		{"get-defaults", no_argument, NULL, 'G'},
 		{"set-threshold", required_argument, NULL, 's'},
-		{"get-threshold", no_argument, NULL, 'g'},
 		{"set-default-threshold", required_argument, NULL, 'S'},
-		{"set-flush", required_argument, NULL, 't'},
-		{"get-flush", no_argument, NULL, 'G'},
-		{"set-default-flush", required_argument, NULL, 'T'},
+		{"set-timeout", required_argument, NULL, 't'},
+		{"set-default-timeout", required_argument, NULL, 'T'},
 		{"stats", no_argument, NULL, 'q'},
 		{"interval", required_argument, NULL, 'i'},
 		{"version", no_argument, NULL, 'V'},
@@ -276,7 +286,7 @@ int main(int argc, char **argv)
 	mod.interval = 1;
 
 	while ((i =
-		getopt_long(argc, argv, "qs:S:t:T:gGi:Vh", longopts,
+		getopt_long(argc, argv, "gGs:S:t:T:qi:Vh", longopts,
 			    NULL)) != -1) {
 		switch (i) {
 		case 'q':
@@ -306,6 +316,8 @@ int main(int argc, char **argv)
 				errx(EXIT_FAILURE, ("Invalid threshold default value: %d"),
 						    mod.threshold_def_val);
 			break;
+		case OPT_SET_FLUSH:
+			warnx(_("%s is deprecated, and will be removed in future, please use %s instead"), "--set-flush", "--set-timeout");
 		case 't':
 			mod.set_timeout = 1;
 			intention_expressed = 1;
@@ -315,6 +327,8 @@ int main(int argc, char **argv)
 				errx(EXIT_FAILURE, _("Invalid set timeout value: %d"),
 						     mod.timeout_val);
 			break;
+		case OPT_SET_FLUSH_DEFAULT:
+			warnx(_("%s is deprecated, and will be removed in future, please use %s instead"), "--set-default-flush", "--set-default-timeout");
 		case 'T':
 			mod.set_threshold_def = 1;
 			intention_expressed = 1;
@@ -324,17 +338,20 @@ int main(int argc, char **argv)
 				errx(EXIT_FAILURE, _("Invalid default time value: %d"),
 						     mod.timeout_def_val);
 			break;
+		case OPT_GET_TRESHHOLD:
+			warnx(_("%s is deprecated, and will be removed in future, please use %s instead"), "--get-threshold", "--get-current");
 		case 'g':
 			mod.get_current = 1;
 			intention_expressed = 1;
 			break;
+		case OPT_GET_FLUSH:
+			warnx(_("%s is deprecated, and will be removed in future, please use %s instead"), "--get-flush", "--get-defaults");
 		case 'G':
 			mod.get_defaults = 1;
 			intention_expressed = 1;
 			break;
 		case 'V':
-			printf(_("%s from %s\n"), program_invocation_short_name,
-			       PACKAGE_STRING);
+			printf(UTIL_LINUX_VERSION);
 			return EXIT_SUCCESS;
 		case 'h':
 			usage(stdout);
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 12/15] cytune: add filename to struct cyclades_control
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
                   ` (10 preceding siblings ...)
  2014-05-04 15:49 ` [PATCH 11/15] cytune: deprecate undescriptive options Sami Kerola
@ 2014-05-04 15:49 ` Sami Kerola
  2014-05-04 15:49 ` [PATCH 13/15] cytune: add noreturn function attributes Sami Kerola
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

This allows point to single structure rather than constantly worry if the
index variable to list is referring to correct location.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/cytune.c | 59 ++++++++++++++++++++++++------------------------------
 1 file changed, 26 insertions(+), 33 deletions(-)

diff --git a/sys-utils/cytune.c b/sys-utils/cytune.c
index 0f97e28..dde793e 100644
--- a/sys-utils/cytune.c
+++ b/sys-utils/cytune.c
@@ -60,6 +60,7 @@
 
 struct cyclades_control {
 	struct cyclades_monitor c;
+	char *filename;
 	int cfile;
 	int maxmax;
 	double maxtran;
@@ -71,7 +72,6 @@ struct cyclades_control *cmon;
 struct cytune_modifiers {
 	int global_argc;
 	int global_optind;
-	char ***global_argv;
 	int cmon_index;
 	unsigned int interval;
 	unsigned long threshold_value;
@@ -124,33 +124,26 @@ static void signal_handler(int sig __attribute__((__unused__)))
 {
 	struct cyclades_control *cc;
 	int argc, local_optind;
-	char **argv;
 	int i, j = 0;
 
 	argc = mod.global_argc;
-	argv = *mod.global_argv;
 	local_optind = mod.global_optind;
 
 	for (i = local_optind; i < argc; i++, j++) {
 		cc = &cmon[j];
 		warnx(_("File %s, For threshold value %lu, Maximum characters in fifo were %d,\n"
 			"and the maximum transfer rate in characters/second was %f"),
-		      argv[i], cc->threshold_value, cc->maxmax, cc->maxtran);
+		      cc->filename, cc->threshold_value, cc->maxmax, cc->maxtran);
 	}
 	exit(EXIT_SUCCESS);
 }
 
-static void print_statistics(struct cyclades_control *mon, struct cytune_modifiers *mod, int init)
+static void print_statistics(struct cyclades_control *cc, int init)
 {
-	struct cyclades_control *cc;
-
-	cc = &mon[mod->cmon_index];
-	if (0 < cc->threshold_value && !init) {
+	if (0 < cc->threshold_value && !init)
 		warnx(_("File %s, For threshold value %lu and timeout value %lu, Maximum characters in fifo were %d,\n"
 			"and the maximum transfer rate in characters/second was %f"),
-		      *mod->global_argv[mod->cmon_index + optind], cc->threshold_value, cc->timeout_value, cc->maxmax,
-		      cc->maxtran);
-	}
+		      cc->filename, cc->threshold_value, cc->timeout_value, cc->maxmax, cc->maxtran);
 	cc->maxmax = 0;
 	cc->maxtran = 0.0;
 	cc->threshold_value = 0;
@@ -171,6 +164,7 @@ static void cyg_get_mon(int fd, struct cyclades_monitor *mon, char *file,
 static void query_tty_stats(int argc, char **argv, struct cytune_modifiers *mod)
 {
 	struct cyclades_monitor cywork;
+	struct cyclades_control *cc;
 	struct timeval lasttime, thistime;
 	int i;
 	double diff;
@@ -191,12 +185,13 @@ static void query_tty_stats(int argc, char **argv, struct cytune_modifiers *mod)
 		err(EXIT_FAILURE, _("gettimeofday failed"));
 
 	for (i = optind; i < argc; i++) {
-		mod->cmon_index = i - optind;
-		cmon[mod->cmon_index].cfile = open(argv[i], O_RDONLY);
-		if (cmon[mod->cmon_index].cfile == -1)
-			err(EXIT_FAILURE, _("cannot open %s"), argv[i]);
-		cyg_get_mon(cmon[mod->cmon_index].cfile, &cmon[mod->cmon_index].c, argv[i], mod);
-		print_statistics(&cmon[mod->cmon_index], mod, 1);
+		cc = &cmon[i - optind];
+		cc->filename = argv[i];
+		cc->cfile = open(cc->filename, O_RDONLY);
+		if (cc->cfile == -1)
+			err(EXIT_FAILURE, _("cannot open %s"), cc->filename);
+		cyg_get_mon(cc->cfile, &cc->c, cc->filename, mod);
+		print_statistics(cc, 1);
 	}
 	while (1) {
 		sleep(mod->interval);
@@ -208,24 +203,24 @@ static void query_tty_stats(int argc, char **argv, struct cytune_modifiers *mod)
 		lasttime.tv_usec = thistime.tv_usec;
 
 		for (i = optind; i < argc; i++) {
-			mod->cmon_index = i - optind;
-			cyg_get_mon(cmon[mod->cmon_index].cfile, &cywork, argv[i], mod);
+			cc = &cmon[i - optind];
+			cyg_get_mon(cc->cfile, &cywork, cc->filename, mod);
 			xfer_rate = cywork.char_count / diff;
 
-			if (mod->threshold_value != cmon[mod->cmon_index].threshold_value ||
-			    mod->timeout_value != cmon[mod->cmon_index].timeout_value) {
-				print_statistics(&cmon[mod->cmon_index], mod, 0);
+			if (mod->threshold_value != cc->threshold_value ||
+			    mod->timeout_value != cc->timeout_value) {
+				print_statistics(cc, 0);
 				/* Note that the summary must come before the
 				 * setting of threshold_value */
-				cmon[mod->cmon_index].threshold_value = mod->threshold_value;
-				cmon[mod->cmon_index].timeout_value = mod->timeout_value;
+				cc->threshold_value = mod->threshold_value;
+				cc->timeout_value = mod->timeout_value;
 			} else {
 				/* Don't record this first cycle after change */
-				if (cmon[mod->cmon_index].maxtran < xfer_rate)
-					cmon[mod->cmon_index].maxtran = xfer_rate;
-				if (cmon[mod->cmon_index].maxmax < 0 ||
-				    (unsigned long)cmon[mod->cmon_index].maxmax < cywork.char_max)
-					cmon[mod->cmon_index].maxmax = cywork.char_max;
+				if (cc->maxtran < xfer_rate)
+					cc->maxtran = xfer_rate;
+				if (cc->maxmax < 0 ||
+				    (unsigned long)cc->maxmax < cywork.char_max)
+					cc->maxmax = cywork.char_max;
 			}
 			printf(_("%s: %lu ints, %lu chars; fifo: %lu thresh, %lu tmout, "
 				 "%lu max, %lu now\n"), argv[i],
@@ -235,8 +230,7 @@ static void query_tty_stats(int argc, char **argv, struct cytune_modifiers *mod)
 			printf(_("   %f int/sec; %f rec (char/sec)\n"),
 			       cywork.int_count / diff, xfer_rate);
 
-			memcpy(&cmon[mod->cmon_index].c, &cywork,
-			       sizeof(struct cyclades_monitor));
+			memcpy(&cc->c, &cywork, sizeof(struct cyclades_monitor));
 		}
 	}
 
@@ -277,7 +271,6 @@ int main(int argc, char **argv)
 
 	/* For signal routine. */
 	mod.global_argc = argc;
-	mod.global_argv = &argv;
 
 	setlocale(LC_ALL, "");
 	bindtextdomain(PACKAGE, LOCALEDIR);
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 13/15] cytune: add noreturn function attributes
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
                   ` (11 preceding siblings ...)
  2014-05-04 15:49 ` [PATCH 12/15] cytune: add filename to struct cyclades_control Sami Kerola
@ 2014-05-04 15:49 ` Sami Kerola
  2014-05-04 15:49 ` [PATCH 14/15] cytune: use matching type in struct cyclades_control with kernel Sami Kerola
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

The query_tty_stats() will exit only when it is signaled to stop.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/cytune.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/sys-utils/cytune.c b/sys-utils/cytune.c
index dde793e..c93951d 100644
--- a/sys-utils/cytune.c
+++ b/sys-utils/cytune.c
@@ -120,7 +120,8 @@ static inline double dtime(struct timeval *tvpnew, struct timeval *tvpold)
 	return diff;
 }
 
-static void signal_handler(int sig __attribute__((__unused__)))
+static void __attribute__((__noreturn__))
+signal_handler(int sig __attribute__((__unused__)))
 {
 	struct cyclades_control *cc;
 	int argc, local_optind;
@@ -135,6 +136,7 @@ static void signal_handler(int sig __attribute__((__unused__)))
 			"and the maximum transfer rate in characters/second was %f"),
 		      cc->filename, cc->threshold_value, cc->maxmax, cc->maxtran);
 	}
+	free(cmon);
 	exit(EXIT_SUCCESS);
 }
 
@@ -161,7 +163,8 @@ static void cyg_get_mon(int fd, struct cyclades_monitor *mon, char *file,
 		err(EXIT_FAILURE, _("cannot get timeout for %s"), file);
 }
 
-static void query_tty_stats(int argc, char **argv, struct cytune_modifiers *mod)
+static void __attribute__((__noreturn__))
+query_tty_stats(int argc, char **argv, struct cytune_modifiers *mod)
 {
 	struct cyclades_monitor cywork;
 	struct cyclades_control *cc;
@@ -233,9 +236,6 @@ static void query_tty_stats(int argc, char **argv, struct cytune_modifiers *mod)
 			memcpy(&cc->c, &cywork, sizeof(struct cyclades_monitor));
 		}
 	}
-
-	free(cmon);
-	return;
 }
 
 int main(int argc, char **argv)
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 14/15] cytune: use matching type in struct cyclades_control with kernel
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
                   ` (12 preceding siblings ...)
  2014-05-04 15:49 ` [PATCH 13/15] cytune: add noreturn function attributes Sami Kerola
@ 2014-05-04 15:49 ` Sami Kerola
  2014-05-04 15:49 ` [PATCH 15/15] cytune: update copyright Sami Kerola
  2014-05-07  7:10 ` [PATCH 00/15] cytune modernization Karel Zak
  15 siblings, 0 replies; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/cytune.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/sys-utils/cytune.c b/sys-utils/cytune.c
index c93951d..d9bc900 100644
--- a/sys-utils/cytune.c
+++ b/sys-utils/cytune.c
@@ -62,7 +62,7 @@ struct cyclades_control {
 	struct cyclades_monitor c;
 	char *filename;
 	int cfile;
-	int maxmax;
+	unsigned long maxmax;
 	double maxtran;
 	unsigned long threshold_value;
 	unsigned long timeout_value;
@@ -132,7 +132,7 @@ signal_handler(int sig __attribute__((__unused__)))
 
 	for (i = local_optind; i < argc; i++, j++) {
 		cc = &cmon[j];
-		warnx(_("File %s, For threshold value %lu, Maximum characters in fifo were %d,\n"
+		warnx(_("File %s, For threshold value %lu, Maximum characters in fifo were %lu,\n"
 			"and the maximum transfer rate in characters/second was %f"),
 		      cc->filename, cc->threshold_value, cc->maxmax, cc->maxtran);
 	}
@@ -143,7 +143,7 @@ signal_handler(int sig __attribute__((__unused__)))
 static void print_statistics(struct cyclades_control *cc, int init)
 {
 	if (0 < cc->threshold_value && !init)
-		warnx(_("File %s, For threshold value %lu and timeout value %lu, Maximum characters in fifo were %d,\n"
+		warnx(_("File %s, For threshold value %lu and timeout value %lu, Maximum characters in fifo were %lu,\n"
 			"and the maximum transfer rate in characters/second was %f"),
 		      cc->filename, cc->threshold_value, cc->timeout_value, cc->maxmax, cc->maxtran);
 	cc->maxmax = 0;
@@ -221,8 +221,7 @@ query_tty_stats(int argc, char **argv, struct cytune_modifiers *mod)
 				/* Don't record this first cycle after change */
 				if (cc->maxtran < xfer_rate)
 					cc->maxtran = xfer_rate;
-				if (cc->maxmax < 0 ||
-				    (unsigned long)cc->maxmax < cywork.char_max)
+				if (cc->maxmax < cywork.char_max)
 					cc->maxmax = cywork.char_max;
 			}
 			printf(_("%s: %lu ints, %lu chars; fifo: %lu thresh, %lu tmout, "
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 15/15] cytune: update copyright
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
                   ` (13 preceding siblings ...)
  2014-05-04 15:49 ` [PATCH 14/15] cytune: use matching type in struct cyclades_control with kernel Sami Kerola
@ 2014-05-04 15:49 ` Sami Kerola
  2014-05-07  7:10 ` [PATCH 00/15] cytune modernization Karel Zak
  15 siblings, 0 replies; 21+ messages in thread
From: Sami Kerola @ 2014-05-04 15:49 UTC (permalink / raw)
  To: util-linux; +Cc: kerolasa

Take the blame of bugs I (hopefully did not) introduce.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/cytune.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sys-utils/cytune.c b/sys-utils/cytune.c
index d9bc900..95db9c9 100644
--- a/sys-utils/cytune.c
+++ b/sys-utils/cytune.c
@@ -35,6 +35,8 @@
   * - added Native Language Support
   * Sun Mar 21 1999 - Arnaldo Carvalho de Melo <acme@conectiva.com.br>
   * - fixed strerr(errno) in gettext calls
+  * May 2014 - Sami Kerola <kerolasa@iki.fi>
+  * - more than 50% rewrite of the code
   */
 
 #include <getopt.h>
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 04/15] cytune: use single loop for setting and getting ioctl() calls
  2014-05-04 15:49 ` [PATCH 04/15] cytune: use single loop for setting and getting ioctl() calls Sami Kerola
@ 2014-05-05  8:24   ` Benno Schulenberg
  0 siblings, 0 replies; 21+ messages in thread
From: Benno Schulenberg @ 2014-05-05  8:24 UTC (permalink / raw)
  To: Sami Kerola; +Cc: Util-Linux


Hi Sami,

On Sun, May 4, 2014, at 17:49, Sami Kerola wrote:
> +		if (set_threshold)
> +			if (ioctl(file, CYSETTHRESH, threshold_val))
> +				err(EXIT_FAILURE, _("cannot set %s to threshold %d"), argv[i], threshold_val);
> +
> +		if (set_threshold_def)
> +			if (ioctl(file, CYSETDEFTHRESH, threshold_def_val))
> +				err(EXIT_FAILURE, _("cannot set %s to threshold %d"), argv[i], threshold_def_val);

Shouldn't the latter message contain the word "default"?

> +		if (set_timeout)
> +			if (ioctl(file, CYSETTIMEOUT, timeout_val))
> +				err(EXIT_FAILURE, _("cannot set %s to time threshold %d"), argv[i], timeout_val);

s/time threshold/timeout/ ?

> +		if (set_timeout_def)
> +			if (ioctl(file, CYSETDEFTIMEOUT, timeout_def_val))
> +				err(EXIT_FAILURE, _("cannot set %s to time threshold %d"), argv[i], timeout_def_val);

s/time threshold/default timeout/ ?

> +		if (get_defaults) {
> +			if (ioctl(file, CYGETDEFTHRESH, &threshold_value))
> +				err(EXIT_FAILURE, _("cannot get threshold for %s"), argv[i]);
> +			if (ioctl(file, CYGETDEFTIMEOUT, &timeout_value))
> +				err(EXIT_FAILURE, _("cannot get timeout for %s"), argv[i]);

Again the words "default"?

Benno

-- 
http://www.fastmail.fm - mmm... Fastmail...


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 02/15] cytune: remove unnecessary variables
  2014-05-04 15:49 ` [PATCH 02/15] cytune: remove unnecessary variables Sami Kerola
@ 2014-05-05  8:29   ` Benno Schulenberg
  0 siblings, 0 replies; 21+ messages in thread
From: Benno Schulenberg @ 2014-05-05  8:29 UTC (permalink / raw)
  To: Sami Kerola; +Cc: Util-Linux


On Sun, May 4, 2014, at 17:49, Sami Kerola wrote:
> -				warnx(_("Invalid threshold default value: %d"),
> -				      threshold_def_val);
> -				errflg++;
> -			}
> +			if (threshold_def_val < 0 || 12 < threshold_def_val)
> +				errx(EXIT_FAILURE, ("Invalid threshold default value: %d"),

s/threshold default/default threshold/

> -				warnx(_("Invalid set timeout value: %d"),
> -				      timeout_val);
> -				errflg++;
> -			}
> +			if (timeout_val < 1 || 255 < timeout_val)
> +				errx(EXIT_FAILURE, _("Invalid set timeout value: %d"),

s/set timeout/timeout/

>  						_("Invalid default timeout value"));
> -			if (timeout_def_val < 0 || 255 < timeout_def_val) {
> -				warnx(_("Invalid default time value: %d"),
> -				      timeout_def_val);
> -				errflg++;
> -			}
> +			if (timeout_def_val < 0 || 255 < timeout_def_val)
> +				errx(EXIT_FAILURE, _("Invalid default time value: %d"),

s/time/timeout/

Benno

-- 
http://www.fastmail.fm - The professional email service


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 01/15] cytune: rename threshold and timeout variables
  2014-05-04 15:49 ` [PATCH 01/15] cytune: rename threshold and timeout variables Sami Kerola
@ 2014-05-05  8:34   ` Benno Schulenberg
  0 siblings, 0 replies; 21+ messages in thread
From: Benno Schulenberg @ 2014-05-05  8:34 UTC (permalink / raw)
  To: Sami Kerola; +Cc: Util-Linux


On Sun, May 4, 2014, at 17:49, Sami Kerola wrote:
> +			threshold_def_val = strtou32_or_err(optarg,
> +						_("Invalid threshold default value"));
> +			if (threshold_def_val < 0 || 12 < threshold_def_val) {
> +				warnx(_("Invalid threshold default value: %d"),

Again, swap "threshold" and "default".

> +						_("Invalid set timeout value"));
> +			if (timeout_val < 1 || 255 < timeout_val) {
> +				warnx(_("Invalid set timeout value: %d"),
> +				      timeout_val);

s/set//

> +						_("Invalid default timeout value"));
> +			if (timeout_def_val < 0 || 255 < timeout_def_val) {
>  				warnx(_("Invalid default time value: %d"),

s/time/timeout/

Benno

-- 
http://www.fastmail.fm - The professional email service


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/15] cytune modernization
  2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
                   ` (14 preceding siblings ...)
  2014-05-04 15:49 ` [PATCH 15/15] cytune: update copyright Sami Kerola
@ 2014-05-07  7:10 ` Karel Zak
  2014-05-07  8:12   ` Sami Kerola
  15 siblings, 1 reply; 21+ messages in thread
From: Karel Zak @ 2014-05-07  7:10 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

On Sun, May 04, 2014 at 04:49:41PM +0100, Sami Kerola wrote:
> Couple days ago Benno Schulenberg mentioned email with subject 'cytune:
> misnamed long options' usage() being a bit misleading that I concurred
> with note that the cytune could probably be improved various ways.  This
> patch set proposes the improvements I had in mind.
> 
> Please notice that I do not have hardware to test the cytune command, so
> testing after the changes did not happen.  All I can say I tried to be
> careful not to break program logic, and hopefully that will work.

 Frankly, I'm a little bit nervous from all the invasive cytune
 changes, because we have no way how to test it. It's fine to change
 warning/error messages, usage() or so, but the another changes
 without tests seem risky.

 The question is if we have to maintain HW specific util, particularly
 when the HW seem rarely available (ebay only?). Maybe the best would
 be to drop cytune.c from u-l and suggest to possible users to use
 old u-l versions or maintain cytune.c outside u-l.

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/15] cytune modernization
  2014-05-07  7:10 ` [PATCH 00/15] cytune modernization Karel Zak
@ 2014-05-07  8:12   ` Sami Kerola
  0 siblings, 0 replies; 21+ messages in thread
From: Sami Kerola @ 2014-05-07  8:12 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On 7 May 2014 08:10, Karel Zak <kzak@redhat.com> wrote:
> On Sun, May 04, 2014 at 04:49:41PM +0100, Sami Kerola wrote:
>> Couple days ago Benno Schulenberg mentioned email with subject 'cytune:
>> misnamed long options' usage() being a bit misleading that I concurred
>> with note that the cytune could probably be improved various ways.  This
>> patch set proposes the improvements I had in mind.
>>
>> Please notice that I do not have hardware to test the cytune command, so
>> testing after the changes did not happen.  All I can say I tried to be
>> careful not to break program logic, and hopefully that will work.
>
>  Frankly, I'm a little bit nervous from all the invasive cytune
>  changes, because we have no way how to test it. It's fine to change
>  warning/error messages, usage() or so, but the another changes
>  without tests seem risky.

How about if I would splitting the cytune changes in two; safe changes
that can be merged and testing todo changes.  The todo changes would be
are archived to a cytune branch in upstream git until (if ever) someone
who has hardware has checked all works.

BTW does anyone in this list have cyclades and time for running sanity
checks?

>  The question is if we have to maintain HW specific util, particularly
>  when the HW seem rarely available (ebay only?). Maybe the best would
>  be to drop cytune.c from u-l and suggest to possible users to use
>  old u-l versions or maintain cytune.c outside u-l.

IMHO dropping the command would be best.

I can either prepare a safe change + testing todo or clean up. Let's wait for
couple days if someone has a good reason why the cytune(8) is still needed.

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2014-05-07  8:12 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-04 15:49 [PATCH 00/15] cytune modernization Sami Kerola
2014-05-04 15:49 ` [PATCH 01/15] cytune: rename threshold and timeout variables Sami Kerola
2014-05-05  8:34   ` Benno Schulenberg
2014-05-04 15:49 ` [PATCH 02/15] cytune: remove unnecessary variables Sami Kerola
2014-05-05  8:29   ` Benno Schulenberg
2014-05-04 15:49 ` [PATCH 03/15] cytune: be consistent with interval data type Sami Kerola
2014-05-04 15:49 ` [PATCH 04/15] cytune: use single loop for setting and getting ioctl() calls Sami Kerola
2014-05-05  8:24   ` Benno Schulenberg
2014-05-04 15:49 ` [PATCH 05/15] cytune: kernel still does not have send_count in cyclades_monitor structure Sami Kerola
2014-05-04 15:49 ` [PATCH 06/15] cytune: prefer sigaction(), and remove unnecessary abstractions Sami Kerola
2014-05-04 15:49 ` [PATCH 07/15] cytune: add cyg_get_mon() to avoid duplicate code Sami Kerola
2014-05-04 15:49 ` [PATCH 08/15] cytune: add structure to hold run time configuration Sami Kerola
2014-05-04 15:49 ` [PATCH 09/15] cytune: pull signal handling and statistic priting apart Sami Kerola
2014-05-04 15:49 ` [PATCH 10/15] cytune: remove unnecessary type casts Sami Kerola
2014-05-04 15:49 ` [PATCH 11/15] cytune: deprecate undescriptive options Sami Kerola
2014-05-04 15:49 ` [PATCH 12/15] cytune: add filename to struct cyclades_control Sami Kerola
2014-05-04 15:49 ` [PATCH 13/15] cytune: add noreturn function attributes Sami Kerola
2014-05-04 15:49 ` [PATCH 14/15] cytune: use matching type in struct cyclades_control with kernel Sami Kerola
2014-05-04 15:49 ` [PATCH 15/15] cytune: update copyright Sami Kerola
2014-05-07  7:10 ` [PATCH 00/15] cytune modernization Karel Zak
2014-05-07  8:12   ` Sami Kerola

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.