[PATCH 0/4] Pull Request

[PATCH 0/4] Pull Request

[J William Piggott]

This patch set address four bugs. Three are related to ISO 8601 formats
and the fourth is a tangentially related bug in hwclock. 

Patch 0002 increases the ISO 8601 buffer macro from 32 to 42 which should
work for the first three, and may be usable in the last four files:
  login-utils/last.c:1039	buffer size 32
  misc-utils/uuidparse.c:231	uses ISO_8601_BUFSIZ + 4
  login-utils/utmpdump.c:94	buffer size 40
  login-utils/lslogins.c:316	buffer size 64
  sys-utils/lsipc.c:1328	buffer size 64
  sys-utils/dmesg.c:887		buffer size 256
  term-utils/script.c:351	uses BUFSIZ (8K on my system)

I haven't tested it on them.

The the final patch adds some common ISO timestamp format masks.

The following changes since commit b41bac08abadbea9bac7a093c995ca53d86c76f1:

  build-sys: move rfkill to /usr/sbin (2017-10-20 14:59:16 +0200)

are available in the git repository at:

  git@github.com:jwpi/util-linux.git 170925

for you to fetch changes up to a9f92c6d1f25f4111f1334bdb2dd96f8b4ccb9ba:

  lib/timeutils: add common ISO timestamp masks (2017-10-21 20:55:01 -0400)

----------------------------------------------------------------
J William Piggott (4):
      hwclock: add iso-8601 overflow check
      lib/timeutils: ISO_8601_BUFSIZ too small
      lib/timeutils: add get_gmtoff()
      lib/timeutils: add common ISO timestamp masks

 include/timeutils.h    | 23 ++++++++++++-----
 lib/timeutils.c        | 70 +++++++++++++++++++++++++++++++++++++++++++++++---
 login-utils/last.c     |  2 +-
 login-utils/lslogins.c |  3 +--
 login-utils/utmpdump.c |  4 +--
 misc-utils/uuidparse.c | 10 ++------
 sys-utils/dmesg.c      |  4 +--
 sys-utils/hwclock.c    | 23 +++++++----------
 sys-utils/lsipc.c      |  2 +-
 sys-utils/rfkill.c     |  8 ++----
 term-utils/script.c    |  8 ++----
 11 files changed, 103 insertions(+), 54 deletions(-)

[PATCH 1/4] hwclock: add iso-8601 overflow check

[J William Piggott]

hwclock wasn't testing for strtimeval_iso() truncation:

/sbin/hwclock --utc --noadjfile --predict --date '7982 years'; echo $?
9999-09-25 19:33:01.000000-0400
0

/sbin/hwclock --utc --noadjfile --predict --date '7983 years'; echo $?
10000-09-25 19:33:10.000000-
0

Patched:
./hwclock --utc --noadjfile --predict --date '7982 years'; echo $?
9999-09-25 19:22:15.000000-0400
0

./hwclock --utc --noadjfile --predict --date '7983 years'; echo $?
hwclock: iso-8601 format truncated
1

Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
 sys-utils/hwclock.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 36b6204b0..c2c20812c 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -551,15 +551,19 @@ set_hardware_clock_exact(const struct hwclock_control *ctl,
 	set_hardware_clock(ctl, newhwtime);
 }
 
-static void
+static int
 display_time(struct timeval hwctime)
 {
 	char buf[ISO_8601_BUFSIZ];
 
-	strtimeval_iso(&hwctime, ISO_8601_DATE|ISO_8601_TIME|ISO_8601_DOTUSEC|
+	if (strtimeval_iso(&hwctime, ISO_8601_DATE|ISO_8601_TIME|ISO_8601_DOTUSEC|
 				 ISO_8601_TIMEZONE|ISO_8601_SPACE,
-				 buf, sizeof(buf));
+				 buf, sizeof(buf))) {
+		warnx(_("iso-8601 format overflow"));
+		return EXIT_FAILURE;
+	}
 	printf("%s\n", buf);
+	return EXIT_SUCCESS;
 }
 
 /*
@@ -931,8 +935,7 @@ manipulate_clock(const struct hwclock_control *ctl, const time_t set_time,
 			printf(_ ("Target date:   %ld\n"), set_time);
 			printf(_ ("Predicted RTC: %ld\n"), hclocktime.tv_sec);
 		}
-		display_time(hclocktime);
-		return EXIT_SUCCESS;
+		return display_time(hclocktime);
 	}
 
 	if (ctl->systz)
@@ -978,7 +981,7 @@ manipulate_clock(const struct hwclock_control *ctl, const time_t set_time,
 		 time_inc(hclocktime, time_diff(startup_time, read_time));
 	}
 	if (ctl->show || ctl->get) {
-		display_time(startup_hclocktime);
+		return display_time(startup_hclocktime);
 	} else if (ctl->set) {
 		set_hardware_clock_exact(ctl, set_time, startup_time);
 		if (!ctl->noadjfile)

[PATCH 2/4] lib/timeutils: ISO_8601_BUFSIZ too small

[J William Piggott]

Although iso-8601 specifies years as 4 digits, it allows
them to be wider.

The current POSIX year width is limited by 'int tm_year'
at 10 digits plus a negative sign.

That, and the possibility of nanosecond time makes the
widest POSIX iso-8601 time 41 characters. Plus the \0
string terminator yields a buffer size of 42.

Before truncated output:
/sbin/hwclock --utc --noadjfile --predict --date '-2147483765 years'
-2147481748-09-25 20:29:45.0000

Patched:
./hwclock --utc --noadjfile --predict --date '-2147483765 years'
-2147481748-09-25 20:17:21.000000-0456

./hwclock --utc --noadjfile --predict --date '-2147483766 years'
hwclock: invalid date '-2147483766 years'

Comparable to coreutils 'date' command:
date -Ins --date '-2147483765 years'
-2147481748-09-25T19:49:31,578899297-0456

date -Ins --date '-2147483766 years'
date: invalid date '-2147483766 years'

The 'date' output illustrates the full 41 character POSIX iso-8601

Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
 include/timeutils.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/timeutils.h b/include/timeutils.h
index 874f853b7..edd42f7fe 100644
--- a/include/timeutils.h
+++ b/include/timeutils.h
@@ -65,7 +65,7 @@ enum {
 	ISO_8601_GMTIME		= (1 << 7)
 };
 
-#define ISO_8601_BUFSIZ	32
+#define ISO_8601_BUFSIZ	42
 
 int strtimeval_iso(struct timeval *tv, int flags, char *buf, size_t bufsz);
 int strtm_iso(struct tm *tm, int flags, char *buf, size_t bufsz);

[PATCH 3/4] lib/timeutils: add get_gmtoff()

[J William Piggott]

This new function returns the GMT offset relative to its
argument. It is used in this patch to fix two bugs:

1) On platforms that the tm struct excludes tm_gmtoff,
   hwclock assumes a one hour DST offset. This can cause
   an incorrect kernel timezone setting. For example:

 Current; with tm_gmtoff illustrates the correct offset:
$ TZ="Australia/Lord_Howe" hwclock --hctosys --test | grep settimeofday
Calling settimeofday(1507494204.192398, -660)

 Current; without tm_gmtoff:
$ TZ="Australia/Lord_Howe" hwclock --hctosys --test | grep settimeofday
Calling settimeofday(1507494249.193852, -690)

 Patched; without tm_gmtoff:
$ TZ="Australia/Lord_Howe" hwclock --hctosys --test | grep settimeofday
Calling settimeofday(1507494260.194208, -660)

2) ISO 8601 'extended' format requires all time elements
   to use a colon (:).

Current invalid ISO 8601:
$ hwclock
2017-10-08 16:25:17.895462-0400

Patched:
$ hwclock
2017-10-08 16:25:34.141895-04:00

Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
 include/timeutils.h |  1 +
 lib/timeutils.c     | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 sys-utils/hwclock.c |  8 +------
 3 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/include/timeutils.h b/include/timeutils.h
index edd42f7fe..e8a261462 100644
--- a/include/timeutils.h
+++ b/include/timeutils.h
@@ -53,6 +53,7 @@ typedef uint64_t nsec_t;
 #define FORMAT_TIMESPAN_MAX 64
 
 int parse_timestamp(const char *t, usec_t *usec);
+int get_gmtoff(const struct tm *tp);
 
 /* flags for strxxx_iso() functions */
 enum {
diff --git a/lib/timeutils.c b/lib/timeutils.c
index d38970c10..adc255c33 100644
--- a/lib/timeutils.c
+++ b/lib/timeutils.c
@@ -341,6 +341,65 @@ int parse_timestamp(const char *t, usec_t *usec)
 	return 0;
 }
 
+/* Returns the difference in seconds between its argument and GMT. If if TP is
+ * invalid or no DST information is available default to UTC, that is, zero.
+ * tzset is called so, for example, 'TZ="UTC" hwclock' will work as expected.
+ * Derived from glibc/time/strftime_l.c
+ */
+int get_gmtoff(const struct tm *tp)
+{
+	if (tp->tm_isdst < 0)
+	return 0;
+
+#if HAVE_TM_GMTOFF
+	return tp->tm_gmtoff;
+#else
+	struct tm tm;
+	struct tm gtm;
+	struct tm ltm = *tp;
+	time_t lt;
+
+	tzset();
+	lt = mktime(&ltm);
+	/* Check if mktime returning -1 is an error or a valid time_t */
+	if (lt == (time_t) -1) {
+		if (! localtime_r(&lt, &tm)
+			|| ((ltm.tm_sec ^ tm.tm_sec)
+			    | (ltm.tm_min ^ tm.tm_min)
+			    | (ltm.tm_hour ^ tm.tm_hour)
+			    | (ltm.tm_mday ^ tm.tm_mday)
+			    | (ltm.tm_mon ^ tm.tm_mon)
+			    | (ltm.tm_year ^ tm.tm_year)))
+			return 0;
+	}
+
+	if (! gmtime_r(&lt, &gtm))
+		return 0;
+
+	/* Calculate the GMT offset, that is, the difference between the
+	 * TP argument (ltm) and GMT (gtm).
+	 *
+	 * Compute intervening leap days correctly even if year is negative.
+	 * Take care to avoid int overflow in leap day calculations, but it's OK
+	 * to assume that A and B are close to each other.
+	 */
+	int a4 = (ltm.tm_year >> 2) + (1900 >> 2) - ! (ltm.tm_year & 3);
+	int b4 = (gtm.tm_year >> 2) + (1900 >> 2) - ! (gtm.tm_year & 3);
+	int a100 = a4 / 25 - (a4 % 25 < 0);
+	int b100 = b4 / 25 - (b4 % 25 < 0);
+	int a400 = a100 >> 2;
+	int b400 = b100 >> 2;
+	int intervening_leap_days = (a4 - b4) - (a100 - b100) + (a400 - b400);
+
+	int years = ltm.tm_year - gtm.tm_year;
+	int days = (365 * years + intervening_leap_days
+		    + (ltm.tm_yday - gtm.tm_yday));
+
+	return (60 * (60 * (24 * days + (ltm.tm_hour - gtm.tm_hour))
+		+ (ltm.tm_min - gtm.tm_min)) + (ltm.tm_sec - gtm.tm_sec));
+#endif
+}
+
 static int format_iso_time(struct tm *tm, suseconds_t usec, int flags, char *buf, size_t bufsz)
 {
 	char *p = buf;
@@ -386,9 +445,14 @@ static int format_iso_time(struct tm *tm, suseconds_t usec, int flags, char *buf
 		p += len;
 	}
 
-	if (flags & ISO_8601_TIMEZONE && strftime(p, bufsz, "%z", tm) <= 0)
+	if (flags & ISO_8601_TIMEZONE) {
+		int tmin  = get_gmtoff(tm) / 60;
+		int zhour = tmin / 60;
+		int zmin  = abs(tmin % 60);
+		len = snprintf(p, bufsz, "%+03d:%02d", zhour,zmin);
+		if (len < 0 || (size_t) len > bufsz)
 		return -1;
-
+	}
 	return 0;
 }
 
diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index c2c20812c..3ac43efee 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -608,13 +608,7 @@ set_system_clock(const struct hwclock_control *ctl,
 	const struct timezone tz_utc = { 0 };
 
 	broken = localtime(&newtime.tv_sec);
-#ifdef HAVE_TM_GMTOFF
-	minuteswest = -broken->tm_gmtoff / 60;	/* GNU extension */
-#else
-	minuteswest = timezone / 60;
-	if (broken->tm_isdst)
-		minuteswest -= 60;
-#endif
+	minuteswest = -get_gmtoff(broken) / 60;
 
 	if (ctl->debug) {
 		if (ctl->hctosys && !ctl->universal)

[PATCH 4/4] lib/timeutils: add common ISO timestamp masks

[J William Piggott]

* Start the ISO format flags at bit 0 instead of bit 1.

* ISO timestamps have date-time-timzone in common, so move
  the TIMEZONE flag to bit 2 causing all timestamp masks
  to have the first three bits set and the last four bits
  as timestamp 'options'.

* Change the 'SPACE' flag to a 'T' flag, because it makes
  the code and comments more concise.

* Add common ISO timestamp masks.

* Implement the ISO timestamp masks in all applicable code
  using the strxxx_iso() functions.

Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
 include/timeutils.h    | 20 ++++++++++++++------
 lib/timeutils.c        |  2 +-
 login-utils/last.c     |  2 +-
 login-utils/lslogins.c |  3 +--
 login-utils/utmpdump.c |  4 +---
 misc-utils/uuidparse.c | 10 ++--------
 sys-utils/dmesg.c      |  4 +---
 sys-utils/hwclock.c    |  4 +---
 sys-utils/lsipc.c      |  2 +-
 sys-utils/rfkill.c     |  8 ++------
 term-utils/script.c    |  8 ++------
 11 files changed, 27 insertions(+), 40 deletions(-)

diff --git a/include/timeutils.h b/include/timeutils.h
index e8a261462..7d5b13b2d 100644
--- a/include/timeutils.h
+++ b/include/timeutils.h
@@ -55,15 +55,23 @@ typedef uint64_t nsec_t;
 int parse_timestamp(const char *t, usec_t *usec);
 int get_gmtoff(const struct tm *tp);
 
-/* flags for strxxx_iso() functions */
+/* flags and masks for strxxx_iso() functions */
 enum {
-	ISO_8601_DATE		= (1 << 1),
-	ISO_8601_TIME		= (1 << 2),
+	ISO_8601_DATE		= (1 << 0),
+	ISO_8601_TIME		= (1 << 1),
+	ISO_8601_TIMEZONE	= (1 << 2),
 	ISO_8601_DOTUSEC	= (1 << 3),
 	ISO_8601_COMMAUSEC	= (1 << 4),
-	ISO_8601_TIMEZONE	= (1 << 5),
-	ISO_8601_SPACE		= (1 << 6),
-	ISO_8601_GMTIME		= (1 << 7)
+	ISO_8601_T		= (1 << 5),
+	ISO_8601_GMTIME		= (1 << 6),
+	ISO_TIMESTAMP		= 0x07,   /* 2017-10-13 17:27:04-04:00 */
+	ISO_TIMESTAMP_T		= 0x27,   /* 2017-10-13T17:27:04-04:00 */
+	ISO_TIMESTAMP_DOT	= 0x0F,   /* 2017-10-13 17:27:04.350452-04:00 */
+	ISO_TIMESTAMP_DOT_T	= 0x2F,   /* 2017-10-13T17:27:04.350452-04:00 */
+	ISO_TIMESTAMP_COMMA	= 0x17,   /* 2017-10-13 17:27:04,350452-04:00 */
+	ISO_TIMESTAMP_COMMA_T	= 0x37,   /* 2017-10-13T17:27:04,350452-04:00 */
+	ISO_TIMESTAMP_COMMA_G	= 0x57,   /* 2017-10-13 21:27:04,350452+00:00 */
+	ISO_TIMESTAMP_COMMA_GT  = 0x77    /* 2017-10-13T21:27:04,350452+00:00 */
 };
 
 #define ISO_8601_BUFSIZ	42
diff --git a/lib/timeutils.c b/lib/timeutils.c
index adc255c33..21d384c58 100644
--- a/lib/timeutils.c
+++ b/lib/timeutils.c
@@ -417,7 +417,7 @@ static int format_iso_time(struct tm *tm, suseconds_t usec, int flags, char *buf
 	if ((flags & ISO_8601_DATE) && (flags & ISO_8601_TIME)) {
 		if (bufsz < 1)
 			return -1;
-		*p++ = (flags & ISO_8601_SPACE) ? ' ' : 'T';
+		*p++ = (flags & ISO_8601_T) ? 'T' : ' ';
 		bufsz--;
 	}
 
diff --git a/login-utils/last.c b/login-utils/last.c
index f989836ba..8d69ed54e 100644
--- a/login-utils/last.c
+++ b/login-utils/last.c
@@ -349,7 +349,7 @@ static int time_formatter(int fmt, char *dst, size_t dlen, time_t *when)
 		ret = rtrim_whitespace((unsigned char *) dst);
 		break;
 	case LAST_TIMEFTM_ISO8601:
-		ret = strtime_iso(when, ISO_8601_DATE|ISO_8601_TIME|ISO_8601_TIMEZONE, dst, dlen);
+		ret = strtime_iso(when, ISO_TIMESTAMP_T, dst, dlen);
 		break;
 	default:
 		abort();
diff --git a/login-utils/lslogins.c b/login-utils/lslogins.c
index 1042b9b41..9a55df880 100644
--- a/login-utils/lslogins.c
+++ b/login-utils/lslogins.c
@@ -333,8 +333,7 @@ static char *make_time(int mode, time_t time)
 				buf, sizeof(buf));
 		break;
 	case TIME_ISO:
-		rc = strtime_iso(&time, ISO_8601_DATE|ISO_8601_TIME|ISO_8601_TIMEZONE,
-				   buf, sizeof(buf));
+		rc = strtime_iso(&time, ISO_TIMESTAMP_T, buf, sizeof(buf));
 		break;
 	case TIME_ISO_SHORT:
 		rc = strtime_iso(&time, ISO_8601_DATE, buf, sizeof(buf));
diff --git a/login-utils/utmpdump.c b/login-utils/utmpdump.c
index 00c44b8db..5cc87834a 100644
--- a/login-utils/utmpdump.c
+++ b/login-utils/utmpdump.c
@@ -102,9 +102,7 @@ static void print_utline(struct utmpx *ut, FILE *out)
 	tv.tv_sec = ut->ut_tv.tv_sec;
 	tv.tv_usec = ut->ut_tv.tv_usec;
 
-	if (strtimeval_iso(&tv,
-			   ISO_8601_DATE | ISO_8601_TIME | ISO_8601_COMMAUSEC |
-			   ISO_8601_TIMEZONE | ISO_8601_GMTIME, time_string,
+	if (strtimeval_iso(&tv, ISO_TIMESTAMP_COMMA_GT, time_string,
 			   sizeof(time_string)) != 0)
 		return;
 	cleanse(ut->ut_id);
diff --git a/misc-utils/uuidparse.c b/misc-utils/uuidparse.c
index 08ba33415..5d2ea7810 100644
--- a/misc-utils/uuidparse.c
+++ b/misc-utils/uuidparse.c
@@ -227,14 +227,8 @@ static void fill_table_row(struct libscols_table *tb, char const *const uuid)
 				char date_buf[ISO_8601_BUFSIZ + 4];
 
 				uuid_time(buf, &tv);
-				strtimeval_iso(&tv,
-					       ISO_8601_DATE |
-						 ISO_8601_TIME |
-						 ISO_8601_COMMAUSEC |
-						 ISO_8601_TIMEZONE |
-						 ISO_8601_SPACE,
-					       date_buf,
-					       sizeof(date_buf));
+				strtimeval_iso(&tv, ISO_TIMESTAMP_COMMA,
+					       date_buf, sizeof(date_buf));
 				str = xstrdup(date_buf);
 			}
 			break;
diff --git a/sys-utils/dmesg.c b/sys-utils/dmesg.c
index b626380f7..8a86c2ccb 100644
--- a/sys-utils/dmesg.c
+++ b/sys-utils/dmesg.c
@@ -844,9 +844,7 @@ static char *iso_8601_time(struct dmesg_control *ctl, struct dmesg_record *rec,
 		.tv_usec = rec->tv.tv_usec
 	};
 
-	if (strtimeval_iso(&tv,	ISO_8601_DATE|ISO_8601_TIME|ISO_8601_COMMAUSEC|
-				ISO_8601_TIMEZONE,
-				buf, bufsz) != 0)
+	if (strtimeval_iso(&tv,	ISO_TIMESTAMP_COMMA_T, buf, bufsz) != 0)
 		return NULL;
 
 	return buf;
diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 3ac43efee..c1eb8b939 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -556,9 +556,7 @@ display_time(struct timeval hwctime)
 {
 	char buf[ISO_8601_BUFSIZ];
 
-	if (strtimeval_iso(&hwctime, ISO_8601_DATE|ISO_8601_TIME|ISO_8601_DOTUSEC|
-				 ISO_8601_TIMEZONE|ISO_8601_SPACE,
-				 buf, sizeof(buf))) {
+	if (strtimeval_iso(&hwctime, ISO_TIMESTAMP_DOT, buf, sizeof(buf))) {
 		warnx(_("iso-8601 format overflow"));
 		return EXIT_FAILURE;
 	}
diff --git a/sys-utils/lsipc.c b/sys-utils/lsipc.c
index e99c861ab..4b3b0c92d 100644
--- a/sys-utils/lsipc.c
+++ b/sys-utils/lsipc.c
@@ -451,7 +451,7 @@ static char *make_time(int mode, time_t time)
 		strtime_short(&time, &now, 0, buf, sizeof(buf));
 		break;
 	case TIME_ISO:
-		strtime_iso(&time, ISO_8601_DATE|ISO_8601_TIME|ISO_8601_TIMEZONE, buf, sizeof(buf));
+		strtime_iso(&time, ISO_TIMESTAMP_T, buf, sizeof(buf));
 		break;
 	default:
 		errx(EXIT_FAILURE, _("unsupported time type"));
diff --git a/sys-utils/rfkill.c b/sys-utils/rfkill.c
index c9559ef48..044624188 100644
--- a/sys-utils/rfkill.c
+++ b/sys-utils/rfkill.c
@@ -253,12 +253,8 @@ static int rfkill_event(void)
 			continue;
 
 		gettimeofday(&tv, NULL);
-		strtimeval_iso(&tv,
-			       ISO_8601_DATE |
-			       ISO_8601_TIME |
-			       ISO_8601_COMMAUSEC |
-			       ISO_8601_TIMEZONE |
-			       ISO_8601_SPACE, date_buf, sizeof(date_buf));
+		strtimeval_iso(&tv, ISO_TIMESTAMP_COMMA, date_buf,
+			       sizeof(date_buf));
 		printf("%s: idx %u type %u op %u soft %u hard %u\n",
 		       date_buf,
 		       event.idx, event.type, event.op, event.soft, event.hard);
diff --git a/term-utils/script.c b/term-utils/script.c
index cf63ab336..f991de14e 100644
--- a/term-utils/script.c
+++ b/term-utils/script.c
@@ -472,9 +472,7 @@ static void do_io(struct script_control *ctl)
 
 
 	if (ctl->typescriptfp) {
-		strtime_iso(&tvec, ISO_8601_DATE | ISO_8601_TIME |
-				ISO_8601_TIMEZONE | ISO_8601_SPACE,
-				buf, sizeof(buf));
+		strtime_iso(&tvec, ISO_TIMESTAMP, buf, sizeof(buf));
 		fprintf(ctl->typescriptfp, _("Script started on %s\n"), buf);
 	}
 	gettime_monotonic(&ctl->oldtime);
@@ -546,9 +544,7 @@ static void do_io(struct script_control *ctl)
 
 	if (ctl->typescriptfp) {
 		tvec = script_time((time_t *)NULL);
-		strtime_iso(&tvec, ISO_8601_DATE | ISO_8601_TIME |
-				ISO_8601_TIMEZONE | ISO_8601_SPACE,
-				buf, sizeof(buf));
+		strtime_iso(&tvec, ISO_TIMESTAMP, buf, sizeof(buf));
 		fprintf(ctl->typescriptfp, _("\nScript done on %s\n"), buf);
 	}
 	done(ctl);

Re: [PATCH 4/4] lib/timeutils: add common ISO timestamp masks

[Karel Zak]

On Sun, Oct 22, 2017 at 08:41:33PM -0400, J William Piggott wrote:
> 
> * Start the ISO format flags at bit 0 instead of bit 1.
> 
> * ISO timestamps have date-time-timzone in common, so move
>   the TIMEZONE flag to bit 2 causing all timestamp masks
>   to have the first three bits set and the last four bits
>   as timestamp 'options'.
> 
> * Change the 'SPACE' flag to a 'T' flag, because it makes
>   the code and comments more concise.
> 
> * Add common ISO timestamp masks.
> 
> * Implement the ISO timestamp masks in all applicable code
>   using the strxxx_iso() functions.
> 
> Signed-off-by: J William Piggott <elseifthen@gmx.com>
> ---
>  include/timeutils.h    | 20 ++++++++++++++------
>  lib/timeutils.c        |  2 +-
>  login-utils/last.c     |  2 +-
>  login-utils/lslogins.c |  3 +--
>  login-utils/utmpdump.c |  4 +---
>  misc-utils/uuidparse.c | 10 ++--------
>  sys-utils/dmesg.c      |  4 +---
>  sys-utils/hwclock.c    |  4 +---
>  sys-utils/lsipc.c      |  2 +-
>  sys-utils/rfkill.c     |  8 ++------
>  term-utils/script.c    |  8 ++------
>  11 files changed, 27 insertions(+), 40 deletions(-)
> 
> diff --git a/include/timeutils.h b/include/timeutils.h
> index e8a261462..7d5b13b2d 100644
> --- a/include/timeutils.h
> +++ b/include/timeutils.h
> @@ -55,15 +55,23 @@ typedef uint64_t nsec_t;
>  int parse_timestamp(const char *t, usec_t *usec);
>  int get_gmtoff(const struct tm *tp);
>  
> -/* flags for strxxx_iso() functions */
> +/* flags and masks for strxxx_iso() functions */
>  enum {
> -	ISO_8601_DATE		= (1 << 1),
> -	ISO_8601_TIME		= (1 << 2),
> +	ISO_8601_DATE		= (1 << 0),
> +	ISO_8601_TIME		= (1 << 1),
> +	ISO_8601_TIMEZONE	= (1 << 2),
>  	ISO_8601_DOTUSEC	= (1 << 3),
>  	ISO_8601_COMMAUSEC	= (1 << 4),
> -	ISO_8601_TIMEZONE	= (1 << 5),
> -	ISO_8601_SPACE		= (1 << 6),
> -	ISO_8601_GMTIME		= (1 << 7)
> +	ISO_8601_T		= (1 << 5),
> +	ISO_8601_GMTIME		= (1 << 6),
> +	ISO_TIMESTAMP		= 0x07,   /* 2017-10-13 17:27:04-04:00 */
> +	ISO_TIMESTAMP_T		= 0x27,   /* 2017-10-13T17:27:04-04:00 */
> +	ISO_TIMESTAMP_DOT	= 0x0F,   /* 2017-10-13 17:27:04.350452-04:00 */
> +	ISO_TIMESTAMP_DOT_T	= 0x2F,   /* 2017-10-13T17:27:04.350452-04:00 */
> +	ISO_TIMESTAMP_COMMA	= 0x17,   /* 2017-10-13 17:27:04,350452-04:00 */
> +	ISO_TIMESTAMP_COMMA_T	= 0x37,   /* 2017-10-13T17:27:04,350452-04:00 */
> +	ISO_TIMESTAMP_COMMA_G	= 0x57,   /* 2017-10-13 21:27:04,350452+00:00 */
> +	ISO_TIMESTAMP_COMMA_GT  = 0x77    /* 2017-10-13T21:27:04,350452+00:00 */
>  };


Would be possible to use ISO_8601_* to compose ISO_TIMESTAMP_*
macros? Your hardcoded magic 0x constants make the header file
less readable. I mean

  ISO_TIMESTAMP = ISO_8601_DATE | ISO_8601_TIME | ISO_8601_TIMEZONE

and it would be also nice to add somehow info about "usec" to the
macro name (see ISO_TIMESTAMP vs. ISO_TIMESTAMP_COMMA).

What about:

    - use "U" prefix for usec times
    - use "GM_" prefix for GMTIME

for example:

   ISO_TIMESTAMP           = ISO_8601_DATE | ISO_8601_TIME | ISO_8601_TIMEZONE,
   ISO_UTIMESTAMP_DOT      = ISO_TIMESTAMP | ISO_8601_DOTUSEC,
   ISO_UTIMESTAMP_COMMA    = ISO_TIMESTAMP | ISO_8601_ISO_8601_COMMAUSEC,
   ISO_GM_UTIMESTAMP_COMMA = ISO_UTIMESTAMP_COMMA | ISO_8601_GMTIME

and so on.

The another nice way how to describe datetime format is to use chars to
complete describe the format

 ISO_YYYYMMDD_HHMISS             = ISO_8601_DATE | ISO_8601_TIME
 ISO_YYYYMMDD_HHMISS_TZ          = ISO_YYYYMMDD_HHMISS | ISO_8601_TIMEZONE
 ISO_YYYYMMDD_HHMISS_DOT_USEC    = ISO_YYYYMMDD_HHMISS | ISO_8601_DOTUSEC
 ISO_YYYYMMDDTHHMISS_COM_USEC    = ISO_YYYYMMDD_HHMISS | ISO_8601_COMUSEC
 ISO_YYYYMMDDTHHMISS_COM_USEC_TZ = ISO_YYYYMMDDTHHMISS_COM_USEC | ISO_8601_TIMEZONE

the macro name is longer, but very descriptive and easy to extend :-)

 Karel


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

Re: [PATCH 4/4] lib/timeutils: add common ISO timestamp masks

[J William Piggott]

On 10/23/2017 05:49 AM, Karel Zak wrote:
> On Sun, Oct 22, 2017 at 08:41:33PM -0400, J William Piggott wrote:
>>
>> * Start the ISO format flags at bit 0 instead of bit 1.
>>
>> * ISO timestamps have date-time-timzone in common, so move
>>   the TIMEZONE flag to bit 2 causing all timestamp masks
>>   to have the first three bits set and the last four bits
>>   as timestamp 'options'.
>>
>> * Change the 'SPACE' flag to a 'T' flag, because it makes
>>   the code and comments more concise.
>>
>> * Add common ISO timestamp masks.
>>
>> * Implement the ISO timestamp masks in all applicable code
>>   using the strxxx_iso() functions.
>>
>> Signed-off-by: J William Piggott <elseifthen@gmx.com>
>> ---
>>  include/timeutils.h    | 20 ++++++++++++++------
>>  lib/timeutils.c        |  2 +-
>>  login-utils/last.c     |  2 +-
>>  login-utils/lslogins.c |  3 +--
>>  login-utils/utmpdump.c |  4 +---
>>  misc-utils/uuidparse.c | 10 ++--------
>>  sys-utils/dmesg.c      |  4 +---
>>  sys-utils/hwclock.c    |  4 +---
>>  sys-utils/lsipc.c      |  2 +-
>>  sys-utils/rfkill.c     |  8 ++------
>>  term-utils/script.c    |  8 ++------
>>  11 files changed, 27 insertions(+), 40 deletions(-)
>>
>> diff --git a/include/timeutils.h b/include/timeutils.h
>> index e8a261462..7d5b13b2d 100644
>> --- a/include/timeutils.h
>> +++ b/include/timeutils.h
>> @@ -55,15 +55,23 @@ typedef uint64_t nsec_t;
>>  int parse_timestamp(const char *t, usec_t *usec);
>>  int get_gmtoff(const struct tm *tp);
>>  
>> -/* flags for strxxx_iso() functions */
>> +/* flags and masks for strxxx_iso() functions */
>>  enum {
>> -	ISO_8601_DATE		= (1 << 1),
>> -	ISO_8601_TIME		= (1 << 2),
>> +	ISO_8601_DATE		= (1 << 0),
>> +	ISO_8601_TIME		= (1 << 1),
>> +	ISO_8601_TIMEZONE	= (1 << 2),
>>  	ISO_8601_DOTUSEC	= (1 << 3),
>>  	ISO_8601_COMMAUSEC	= (1 << 4),
>> -	ISO_8601_TIMEZONE	= (1 << 5),
>> -	ISO_8601_SPACE		= (1 << 6),
>> -	ISO_8601_GMTIME		= (1 << 7)
>> +	ISO_8601_T		= (1 << 5),
>> +	ISO_8601_GMTIME		= (1 << 6),
>> +	ISO_TIMESTAMP		= 0x07,   /* 2017-10-13 17:27:04-04:00 */
>> +	ISO_TIMESTAMP_T		= 0x27,   /* 2017-10-13T17:27:04-04:00 */
>> +	ISO_TIMESTAMP_DOT	= 0x0F,   /* 2017-10-13 17:27:04.350452-04:00 */
>> +	ISO_TIMESTAMP_DOT_T	= 0x2F,   /* 2017-10-13T17:27:04.350452-04:00 */
>> +	ISO_TIMESTAMP_COMMA	= 0x17,   /* 2017-10-13 17:27:04,350452-04:00 */
>> +	ISO_TIMESTAMP_COMMA_T	= 0x37,   /* 2017-10-13T17:27:04,350452-04:00 */
>> +	ISO_TIMESTAMP_COMMA_G	= 0x57,   /* 2017-10-13 21:27:04,350452+00:00 */
>> +	ISO_TIMESTAMP_COMMA_GT  = 0x77    /* 2017-10-13T21:27:04,350452+00:00 */
>>  };
> 
> 
> Would be possible to use ISO_8601_* to compose ISO_TIMESTAMP_*
> macros? Your hardcoded magic 0x constants make the header file
> less readable. I mean
> 
>   ISO_TIMESTAMP = ISO_8601_DATE | ISO_8601_TIME | ISO_8601_TIMEZONE

We could eliminate wrapping by dropping the unnecessary 8601. Using the
longest example:

	ISO_TIMESTAMP_COMMA_GT = ISO_TIMESTAMP_COMMA | ISO_GMTIME | ISO_T

> 
> and it would be also nice to add somehow info about "usec" to the
> macro name (see ISO_TIMESTAMP vs. ISO_TIMESTAMP_COMMA).

8601 only uses comma or dot for the radix character; so 'COMMA/DOT'
imply the presence of a fractional part.

> What about:
> 
>     - use "U" prefix for usec times
>     - use "GM_" prefix for GMTIME
> 
> for example:
> 
>    ISO_TIMESTAMP           = ISO_8601_DATE | ISO_8601_TIME | ISO_8601_TIMEZONE,
>    ISO_UTIMESTAMP_DOT      = ISO_TIMESTAMP | ISO_8601_DOTUSEC,
>    ISO_UTIMESTAMP_COMMA    = ISO_TIMESTAMP | ISO_8601_ISO_8601_COMMAUSEC,
>    ISO_GM_UTIMESTAMP_COMMA = ISO_UTIMESTAMP_COMMA | ISO_8601_GMTIME
> 
> and so on.

IMO the macros are easier to remember, write, and read, if they all
begin the same and the optional parts are suffixes. All of the masks are
basic ISO Timestamps (date time timezone), the rest are minor variations.

> 
> The another nice way how to describe datetime format is to use chars to
> complete describe the format
> 
>  ISO_YYYYMMDD_HHMISS             = ISO_8601_DATE | ISO_8601_TIME
>  ISO_YYYYMMDD_HHMISS_TZ          = ISO_YYYYMMDD_HHMISS | ISO_8601_TIMEZONE
>  ISO_YYYYMMDD_HHMISS_DOT_USEC    = ISO_YYYYMMDD_HHMISS | ISO_8601_DOTUSEC
>  ISO_YYYYMMDDTHHMISS_COM_USEC    = ISO_YYYYMMDD_HHMISS | ISO_8601_COMUSEC
>  ISO_YYYYMMDDTHHMISS_COM_USEC_TZ = ISO_YYYYMMDDTHHMISS_COM_USEC | ISO_8601_TIMEZONE
> 
> the macro name is longer, but very descriptive and easy to extend :-)

That notation works well when necessary, but an 8601 timestamp is so
specific that duplicating it in the macro name is redundant and, I
think, visually mesmerizing.

> 
>  Karel
> 
> 

Re: [PATCH 4/4] lib/timeutils: add common ISO timestamp masks

[Karel Zak]

On Mon, Oct 23, 2017 at 03:54:19PM -0400, J William Piggott wrote:
> > Would be possible to use ISO_8601_* to compose ISO_TIMESTAMP_*
> > macros? Your hardcoded magic 0x constants make the header file
> > less readable. I mean
> > 
> >   ISO_TIMESTAMP = ISO_8601_DATE | ISO_8601_TIME | ISO_8601_TIMEZONE
> 
> We could eliminate wrapping by dropping the unnecessary 8601. Using the
> longest example:
> 
> 	ISO_TIMESTAMP_COMMA_GT = ISO_TIMESTAMP_COMMA | ISO_GMTIME | ISO_T
 
OK, although ISO_T seems pretty generic :-)

> > and it would be also nice to add somehow info about "usec" to the
> > macro name (see ISO_TIMESTAMP vs. ISO_TIMESTAMP_COMMA).
> 
> 8601 only uses comma or dot for the radix character; so 'COMMA/DOT'
> imply the presence of a fractional part.
> 
> > What about:
> > 
> >     - use "U" prefix for usec times
> >     - use "GM_" prefix for GMTIME
> > 
> > for example:
> > 
> >    ISO_TIMESTAMP           = ISO_8601_DATE | ISO_8601_TIME | ISO_8601_TIMEZONE,
> >    ISO_UTIMESTAMP_DOT      = ISO_TIMESTAMP | ISO_8601_DOTUSEC,
> >    ISO_UTIMESTAMP_COMMA    = ISO_TIMESTAMP | ISO_8601_ISO_8601_COMMAUSEC,
> >    ISO_GM_UTIMESTAMP_COMMA = ISO_UTIMESTAMP_COMMA | ISO_8601_GMTIME
> > 
> > and so on.
> 
> IMO the macros are easier to remember, write, and read, if they all
> begin the same and the optional parts are suffixes. All of the masks are
> basic ISO Timestamps (date time timezone), the rest are minor variations.

I have a problem if I have to use more suffixes, the order of the
suffixes is not obvious. The idea behind ISO_8601_* with in code has
been to keep it readable:

    strtime_iso(ISO_8601_DATE | ISO_8601_TIME | ISO_8601_TIMEZONE)

is so easy to read...

    Karel

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

Re: [PATCH 4/4] lib/timeutils: add common ISO timestamp masks

[J William Piggott]

On 10/24/2017 05:37 AM, Karel Zak wrote:
> On Mon, Oct 23, 2017 at 03:54:19PM -0400, J William Piggott wrote:
>>> Would be possible to use ISO_8601_* to compose ISO_TIMESTAMP_*
>>> macros? Your hardcoded magic 0x constants make the header file
>>> less readable. I mean
>>>
>>>   ISO_TIMESTAMP = ISO_8601_DATE | ISO_8601_TIME | ISO_8601_TIMEZONE
>>
>> We could eliminate wrapping by dropping the unnecessary 8601. Using the
>> longest example:
>>
>> 	ISO_TIMESTAMP_COMMA_GT = ISO_TIMESTAMP_COMMA | ISO_GMTIME | ISO_T
>  
> OK, although ISO_T seems pretty generic :-)
> 
>>> and it would be also nice to add somehow info about "usec" to the
>>> macro name (see ISO_TIMESTAMP vs. ISO_TIMESTAMP_COMMA).
>>
>> 8601 only uses comma or dot for the radix character; so 'COMMA/DOT'
>> imply the presence of a fractional part.
>>
>>> What about:
>>>
>>>     - use "U" prefix for usec times
>>>     - use "GM_" prefix for GMTIME
>>>
>>> for example:
>>>
>>>    ISO_TIMESTAMP           = ISO_8601_DATE | ISO_8601_TIME | ISO_8601_TIMEZONE,
>>>    ISO_UTIMESTAMP_DOT      = ISO_TIMESTAMP | ISO_8601_DOTUSEC,
>>>    ISO_UTIMESTAMP_COMMA    = ISO_TIMESTAMP | ISO_8601_ISO_8601_COMMAUSEC,
>>>    ISO_GM_UTIMESTAMP_COMMA = ISO_UTIMESTAMP_COMMA | ISO_8601_GMTIME
>>>
>>> and so on.
>>
>> IMO the macros are easier to remember, write, and read, if they all
>> begin the same and the optional parts are suffixes. All of the masks are
>> basic ISO Timestamps (date time timezone), the rest are minor variations.
> 
> I have a problem if I have to use more suffixes, the order of the
> suffixes is not obvious. The idea behind ISO_8601_* with in code has
> been to keep it readable:

>From a readability standpoint the order doesn't matter; the order is set
by the standard. The reader only needs to know what the suffixes mean.
There are only four: COMMA, DOT, T, and G. Their meanings seem clear to me.

The current implementation puts the items out of order as well, for
example uuidparse.c has: 
ISO_8601_DATE |
ISO_8601_TIME |
ISO_8601_COMMAUSEC |
ISO_8601_TIMEZONE |
ISO_8601_SPACE,

SPACE is out of order, but it doesn't matter. The standard defines
where it goes. As long as the reader understands that SPACE defines the
date and time delimiter all is good.

Anyway, I'm not clear on what you want me to do. You didn't comment on
my idea that naming the radix point implies usec. Why define a radix
point if there is no fractional part, it makes no sense. If you agree
with that, then your only remaining change request is moving the _G
suffix to _GM prefix.  Is that the only change you want? If yes, then
I'll make it so. IMO, it degrades readability, but you have the hammer.


> 
>     strtime_iso(ISO_8601_DATE | ISO_8601_TIME | ISO_8601_TIMEZONE)
> 
> is so easy to read...
> 
>     Karel
> 

Re: [PATCH 4/4] lib/timeutils: add common ISO timestamp masks

[Karel Zak]

On Tue, Oct 24, 2017 at 12:19:59PM -0400, J William Piggott wrote:
> Anyway, I'm not clear on what you want me to do. You didn't comment on
> my idea that naming the radix point implies usec.

Yes, it implies usec, the question is if such thing is obvious for a
random reader.

> Why define a radix
> point if there is no fractional part, it makes no sense. If you agree
> with that, then your only remaining change request is moving the _G
> suffix to _GM prefix.  Is that the only change you want? If yes, then
> I'll make it so. IMO, it degrades readability, but you have the hammer.

Just brainstorming... the current situation is not so bad from my point 
of view :-)

Anyway I don't see a problem to support complete macros ISO_TIMESTAMP_
as you suggested, but please keep it without magic number with in the
header files. The _G suffix is fine. Send patch.

It would be also nice to use all the complex macros in lib/timeutils.c
test program.

    Karel

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

[PATCH 0/4] Pull Request

[J William Piggott]

The following changes since commit 44d753407d6b751f022ef234c85785ccd99c5590:

  tests: unlocks on failed ts_scsi_debug_init (2017-12-07 15:08:29 +0100)

are available in the git repository at:

  git@github.com:jwpi/util-linux.git 171121

for you to fetch changes up to 6cdc7b9c02b251120eb014c4dbc2387d47e7fb46:

  lib/timeutils.c: warn format_iso_time() overflow (2017-12-09 18:43:29 -0500)

----------------------------------------------------------------
J William Piggott (4):
      lib/timeutils.c: bug fix Segmentation fault
      lib/timeutils.c:strxxx_iso: test conversion errors
      lib/timeutils.c:strxxx_iso: do not wrap tm_year
      lib/timeutils.c: warn format_iso_time() overflow

 lib/timeutils.c     | 46 +++++++++++++++++++++++++++++++---------------
 sys-utils/hwclock.c |  5 ++---
 2 files changed, 33 insertions(+), 18 deletions(-)

Re: [PATCH 0/4] Pull Request

[Karel Zak]

On Sun, Dec 10, 2017 at 10:47:04AM -0500, J William Piggott wrote:
> J William Piggott (4):
>       lib/timeutils.c: bug fix Segmentation fault
>       lib/timeutils.c:strxxx_iso: test conversion errors
>       lib/timeutils.c:strxxx_iso: do not wrap tm_year
>       lib/timeutils.c: warn format_iso_time() overflow


Merged, thanks!

    Karel

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

[PATCH 0/4] Pull Request

[J William Piggott]

The following changes since commit 8ffa3b651d7e74acba8f1d831b7f68fdb3c66aae:

  libfdisk: make fdisk compliant to UEFI/GPT specification on PMBR (2017-07-14 12:48:18 +0200)

are available in the git repository at:

  git@github.com:jwpi/util-linux.git 170711

for you to fetch changes up to c26ddc568f3fb26b45a22cbf89d3294113e70377:

  hwclock: improve RTC epoch messages (2017-07-16 08:41:54 -0400)

----------------------------------------------------------------
J William Piggott (4):
      hwclock: --epoch presence test fails
      hwclock: remove dead ioctl check
      hwclock: improve RTC epoch messages
      hwclock: improve RTC epoch messages

 sys-utils/hwclock-rtc.c | 56 +++++++++++++++++++------------------------------
 sys-utils/hwclock.8.in  |  3 +++
 sys-utils/hwclock.c     | 24 +++++++--------------
 sys-utils/hwclock.h     |  2 +-
 4 files changed, 32 insertions(+), 53 deletions(-)

Re: [PATCH 0/4] Pull Request

[Karel Zak]

On Sun, Jul 16, 2017 at 01:46:38PM -0400, J William Piggott wrote:
>  sys-utils/hwclock-rtc.c | 56 +++++++++++++++++++------------------------------
>  sys-utils/hwclock.8.in  |  3 +++
>  sys-utils/hwclock.c     | 24 +++++++--------------
>  sys-utils/hwclock.h     |  2 +-
>  4 files changed, 32 insertions(+), 53 deletions(-)

Applied, thanks.

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

[PATCH 0/4] pull request

[J William Piggott]

* Rudi didn't like cluttering boilerplate.c with seldom used USAGE_*
  constants and I don't disagree with that point. I do think they should
  be in the file for new contributor discovery purposes. As a compromise
  I added the missing ones as a comment.

* the newly added USAGE_COLUMNS could be used in disk-utils/fdisk-list.c:432
  if Karel wants to drop the ' (for -o)'. I did not change it.

* I couldn't test the blkzone change to USAGE_COMMANDS, but I expect it to
  be okay.
   configure: WARNING: linux/blkzoned.h header not found; not building blkzone


The following changes since commit 2a14beb4e9c6cdf4466993741d86e45dd57ddef3:

  agetty: fix login name DEL/CTRL^U issue (2017-06-23 14:26:47 +0200)

are available in the git repository at:

  git@github.com:jwpi/util-linux.git 170622

for you to fetch changes up to 7948117da5654311dba59b256d9a017d56877592:

  Docs: move option naming to howto-contribute.txt (2017-06-24 15:22:49 -0400)

----------------------------------------------------------------
J William Piggott (4):
      include/c.h: add USAGE_COMMANDS and USAGE_COLUMNS
      Docs: add a comment for constants to boilerplate.c
      Docs: update howto-usage-function.txt
      Docs: move option naming to howto-contribute.txt

 Documentation/boilerplate.c            |  8 +--
 Documentation/howto-contribute.txt     | 25 +++++++++
 Documentation/howto-usage-function.txt | 97 +++++++++++++---------------------
 disk-utils/sfdisk.c                    |  2 +-
 include/c.h                            |  4 +-
 login-utils/lslogins.c                 |  6 +--
 misc-utils/findmnt.c                   |  3 +-
 sys-utils/blkzone.c                    |  2 +-
 sys-utils/lscpu.c                      |  3 +-
 sys-utils/lsmem.c                      |  5 +-
 sys-utils/wdctl.c                      |  3 +-
 11 files changed, 78 insertions(+), 80 deletions(-)

Re: [PATCH 0/4] pull request

[Karel Zak]

On Sun, Jun 25, 2017 at 05:45:56PM -0400, J William Piggott wrote:
> 
> * Rudi didn't like cluttering boilerplate.c with seldom used USAGE_*
>   constants and I don't disagree with that point. I do think they should
>   be in the file for new contributor discovery purposes. As a compromise
>   I added the missing ones as a comment.
> 
> * the newly added USAGE_COLUMNS could be used in disk-utils/fdisk-list.c:432
>   if Karel wants to drop the ' (for -o)'. I did not change it.

I think USAGE_COLUMNS should be good enough everywhere and the
additional notes (like ' (for -o)') are unnecessary.

Maybe we can change the text to "Available output columns:" to make it
more specific for readers.

    Karel


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

Re: [PATCH 0/4] pull request

[Karel Zak]

On Mon, Jun 26, 2017 at 10:27:44AM +0200, Karel Zak wrote:
> On Sun, Jun 25, 2017 at 05:45:56PM -0400, J William Piggott wrote:
> > 
> > * Rudi didn't like cluttering boilerplate.c with seldom used USAGE_*
> >   constants and I don't disagree with that point. I do think they should
> >   be in the file for new contributor discovery purposes. As a compromise
> >   I added the missing ones as a comment.
> > 
> > * the newly added USAGE_COLUMNS could be used in disk-utils/fdisk-list.c:432
> >   if Karel wants to drop the ' (for -o)'. I did not change it.
> 
> I think USAGE_COLUMNS should be good enough everywhere and the
> additional notes (like ' (for -o)') are unnecessary.
> 
> Maybe we can change the text to "Available output columns:" to make it
> more specific for readers.

 Implemented & pull request merged. Thanks.

    Karel

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

Re: [PATCH 0/4] pull request

[J William Piggott]

On 06/26/2017 04:27 AM, Karel Zak wrote:
> On Sun, Jun 25, 2017 at 05:45:56PM -0400, J William Piggott wrote:
>>
>> * Rudi didn't like cluttering boilerplate.c with seldom used USAGE_*
>>   constants and I don't disagree with that point. I do think they should
>>   be in the file for new contributor discovery purposes. As a compromise
>>   I added the missing ones as a comment.
>>
>> * the newly added USAGE_COLUMNS could be used in disk-utils/fdisk-list.c:432
>>   if Karel wants to drop the ' (for -o)'. I did not change it.
> 
> I think USAGE_COLUMNS should be good enough everywhere and the
> additional notes (like ' (for -o)') are unnecessary.
> 
> Maybe we can change the text to "Available output columns:" to make it
> more specific for readers.

Since we cannot guarantee that the 'output' option name will always be
available. It might be better to stay generic with the header and
instead create the reference in the option description like:

 -o, --output <list>    columns to display (see Columns:)

 Columns:
      SOURCE  source device
      ...

This would also keep the Columns header format consistent with the others.

On a somewhat related note; in recent discussion I've been trying to
promote the idea of using consistent language. For example, with all of
the synonyms: output, show, to stdout, display, print, etc. I think it
would be helpful to both users and translators to choose one to be used
consistently. Especially for technical writing it is important to use
consistent terms.

An idea from the Linux man-pages project is that man-pages.7 has a list
of preferred terms/spellings; perhaps util-linux could have a list of
preferred terms to be used in documentation and message strings?

We also might want to consider replacing Documentation/howto-man-page.txt
with a link to, or a copy of, man-pages.7. Michael and company have
added a lot to it in recent times.


> 
>     Karel
> 
> 

Re: [PATCH 0/4] pull request

[Karel Zak]

On Mon, Jun 26, 2017 at 09:47:42AM -0400, J William Piggott wrote:
> 
> 
> On 06/26/2017 04:27 AM, Karel Zak wrote:
> > On Sun, Jun 25, 2017 at 05:45:56PM -0400, J William Piggott wrote:
> >>
> >> * Rudi didn't like cluttering boilerplate.c with seldom used USAGE_*
> >>   constants and I don't disagree with that point. I do think they should
> >>   be in the file for new contributor discovery purposes. As a compromise
> >>   I added the missing ones as a comment.
> >>
> >> * the newly added USAGE_COLUMNS could be used in disk-utils/fdisk-list.c:432
> >>   if Karel wants to drop the ' (for -o)'. I did not change it.
> > 
> > I think USAGE_COLUMNS should be good enough everywhere and the
> > additional notes (like ' (for -o)') are unnecessary.
> > 
> > Maybe we can change the text to "Available output columns:" to make it
> > more specific for readers.
> 
> Since we cannot guarantee that the 'output' option name will always be
> available.

I have already pushed "Available output columns:" :-) 

The "output" does not mean any option in this case.

> It might be better to stay generic with the header and
> instead create the reference in the option description like:
> 
>  -o, --output <list>    columns to display (see Columns:)
> 
>  Columns:
>       SOURCE  source device
>       ...

Anyway, I like this "see Columns:" idea. Go ahead and send patch.

> On a somewhat related note; in recent discussion I've been trying to
> promote the idea of using consistent language. For example, with all of
> the synonyms: output, show, to stdout, display, print, etc. I think it
> would be helpful to both users and translators to choose one to be used
> consistently. Especially for technical writing it is important to use
> consistent terms.
> 
> An idea from the Linux man-pages project is that man-pages.7 has a list
> of preferred terms/spellings; perhaps util-linux could have a list of
> preferred terms to be used in documentation and message strings?

Sounds good. (but I hope we will have no endless "bike-shed
color" discussion about the terms)

> We also might want to consider replacing Documentation/howto-man-page.txt
> with a link to, or a copy of, man-pages.7. Michael and company have
> added a lot to it in recent times.

Documentation/howto-man-page.txt is a template, if you want to write new
tool than it should be good enough to cp(1) this template and use it.
I'd like to keep it there.

We can rename the file to Documentation/boilerplate.1 and rewrite
Documentation/howto-man-page.txt to the real how-to with link to the
Michal's man-pages.7.

    Karel

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

[PATCH 0/4] pull request

[J William Piggott]

docs: various changes; see patches for details.

The following changes since commit 3947ca4ca9737d830f54658ef353f5626c0d0282:

  build-sys: ncurses headers cleanup (2017-05-31 11:01:46 +0200)

are available in the git repository at:

  git@github.com:jwpi/util-linux.git 170529

for you to fetch changes up to 80008bcae9a1aed3d38507a319155a69c4414509:

  docs: move source-code-management.txt to README (2017-05-31 11:36:47 -0400)

----------------------------------------------------------------
J William Piggott (4):
      docs: update v2.30-ReleaseNotes
      docs: update howto-contribute.txt
      docs: update howto-contribute.txt
      docs: move source-code-management.txt to README

 Documentation/howto-contribute.txt        | 201 +++++++++++++++---------------
 Documentation/release-schedule.txt        |   2 +-
 Documentation/releases/v2.30-ReleaseNotes |  38 +++---
 Documentation/source-code-management.txt  |  38 ------
 README                                    | 121 +++++++++++++-----
 5 files changed, 215 insertions(+), 185 deletions(-)
 delete mode 100644 Documentation/source-code-management.txt

Re: [PATCH 0/4] pull request

[Karel Zak]

On Wed, May 31, 2017 at 02:45:23PM -0400, J William Piggott wrote:
>   git@github.com:jwpi/util-linux.git 170529

Applied, thanks.

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

[PATCH 0/4] Pull Request

[J William Piggott]

 sys-utils/hwclock.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

The following changes since commit 85bfb519afcbccccb63849b1a348dde76ff6bb83:

  switch_root: unlink files without _DIRENT_HAVE_D_TYPE (2017-04-26 11:23:50 +0200)

are available in the git repository at:

  git@github.com:jwpi/util-linux.git 170419

for you to fetch changes up to 57415653a667cf2442d019f62287534931ab3da4:

  hwclock: use a consistent name for --predict (2017-04-26 23:19:56 -0400)

----------------------------------------------------------------
J William Piggott (4):
      hwclock: extra messages for debug only
      hwclock: make clock test mode message consistent
      hwclock: remove unneeded braces
      hwclock: use a consistent name for --predict

 sys-utils/hwclock.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

Re: [PATCH 0/4] Pull Request

[Karel Zak]

On Wed, Apr 26, 2017 at 11:29:40PM -0400, J William Piggott wrote:
>       hwclock: extra messages for debug only
>       hwclock: make clock test mode message consistent
>       hwclock: remove unneeded braces
>       hwclock: use a consistent name for --predict

Applied, thanks.

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