[PATCH 00/12] pull request

[PATCH 00/12] pull request

[J William Piggott]

The following changes since commit 998ae28704281303b9131c717ec30e39ca8d62f8:

  Merge branch 'some-fixes' of https://github.com/rudimeier/util-linux (2017-06-16 10:50:24 +0200)

are available in the git repository at:

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

for you to fetch changes up to f7aaa75d6e9b70564114d20fbe2163c7c989fa55:

  hwclock: remove unused stdarg.h (2017-06-18 20:21:27 -0400)

----------------------------------------------------------------
J William Piggott (12):
      hwclock: remove dead code in usage()
      hwclock: update usage() to util-linux style
      hwclock: update usage() FILE name
      hwclock: add usage() functions heading
      include: update pathnames.h
      hwclock: use RTC in help output
      hwclock: update --help content and grammar
      hwclock: slice up the usage text
      hwclock: add --update-drift check
      Docs: update howto-usage-function.txt
      hwclock: remove unused usage() FILE argument
      hwclock: remove unused stdarg.h

 Documentation/boilerplate.c            |   3 +
 Documentation/howto-usage-function.txt |  12 ++--
 include/c.h                            |   7 ++-
 include/pathnames.h                    |  21 +++----
 sys-utils/hwclock.8.in                 |   3 +-
 sys-utils/hwclock.c                    | 109 +++++++++++++--------------------
 6 files changed, 67 insertions(+), 88 deletions(-)

[PATCH 01/12] hwclock: remove dead code in usage()

[J William Piggott]

Commit 677ec86 caused usage() to be called only by --help.

So remove the now dead code from usage().

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

diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 55f54f5..4782318 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -1199,24 +1199,9 @@ static void out_version(void)
 	printf(UTIL_LINUX_VERSION);
 }
 
-/*
- * usage - Output (error and) usage information
- *
- * This function is called both directly from main to show usage information
- * and as fatal function from shhopt if some argument is not understood. In
- * case of normal usage info FMT should be NULL. In that case the info is
- * printed to stdout. If FMT is given usage will act like fprintf( stderr,
- * fmt, ... ), show a usage information and terminate the program
- * afterwards.
- */
 static void __attribute__((__noreturn__))
-usage(const struct hwclock_control *ctl, const char *fmt, ...)
+usage(const struct hwclock_control *ctl, FILE *usageto)
 {
-	FILE *usageto;
-	va_list ap;
-
-	usageto = fmt ? stderr : stdout;
-
 	fputs(USAGE_HEADER, usageto);
 	fputs(_(" hwclock [function] [option...]\n"), usageto);
 
@@ -1263,14 +1248,7 @@ usage(const struct hwclock_control *ctl, const char *fmt, ...)
 	fputs(_("     --test           do not update anything, just show what would happen\n"
 		" -D, --debug          debugging mode\n" "\n"), usageto);
 
-	if (fmt) {
-		va_start(ap, fmt);
-		vfprintf(usageto, fmt, ap);
-		va_end(ap);
-	}
-
-	fflush(usageto);
-	hwclock_exit(ctl, fmt ? EX_USAGE : EX_OK);
+	hwclock_exit(ctl, EXIT_SUCCESS);
 }
 
 /*
@@ -1475,7 +1453,7 @@ int main(int argc, char **argv)
 			out_version();
 			return 0;
 		case 'h':			/* --help */
-			usage(&ctl, NULL);
+			usage(&ctl, stdout);
 		default:
 			errtryhelp(EXIT_FAILURE);
 		}

[PATCH 02/12] hwclock: update usage() to util-linux style

[J William Piggott]

Update usage() according to:
  Documentation/howto-usage-function.txt
  Documentation/boilerplate.c

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

diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 4782318..d2f8ad1 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -1206,11 +1206,11 @@ usage(const struct hwclock_control *ctl, FILE *usageto)
 	fputs(_(" hwclock [function] [option...]\n"), usageto);
 
 	fputs(USAGE_SEPARATOR, usageto);
-	fputs(_("Query or set the hardware clock.\n"), usageto);
+	fputs(_(" Query or set the hardware clock\n"), usageto);
 
-	fputs(_("\nFunctions:\n"), usageto);
-	fputs(_(" -h, --help           show this help text and exit\n"
-		" -r, --show           read hardware clock and print result\n"
+	fputs(USAGE_SEPARATOR, usageto);
+	fputs(_("Functions:\n"), usageto);
+	fputs(_(" -r, --show           read hardware clock and print result\n"
 		"     --get            read hardware clock and print drift corrected result\n"
 		"     --set            set the RTC to the time given with --date\n"), usageto);
 	fputs(_(" -s, --hctosys        set the system time from the hardware clock\n"
@@ -1223,8 +1223,7 @@ usage(const struct hwclock_control *ctl, FILE *usageto)
 		"     --setepoch       set the kernel's hardware clock epoch value to the \n"
 		"                        value given with --epoch\n"), usageto);
 #endif
-	fputs(_("     --predict        predict RTC reading at time given with --date\n"
-		" -V, --version        display version information and exit\n"), usageto);
+	fputs(_("     --predict        predict RTC reading at time given with --date\n"), usageto);
 
 	fputs(USAGE_OPTIONS, usageto);
 	fputs(_(" -u, --utc            the hardware clock is kept in UTC\n"
@@ -1246,8 +1245,11 @@ usage(const struct hwclock_control *ctl, FILE *usageto)
 		"     --adjfile <file> specifies the path to the adjust file;\n"
 		"                        the default is %1$s\n"), _PATH_ADJTIME);
 	fputs(_("     --test           do not update anything, just show what would happen\n"
-		" -D, --debug          debugging mode\n" "\n"), usageto);
-
+		" -D, --debug          debugging mode\n"), usageto);
+	fputs(USAGE_SEPARATOR, usageto);
+	fputs(USAGE_HELP, usageto);
+	fputs(USAGE_VERSION, usageto);
+	fprintf(usageto, USAGE_MAN_TAIL("hwclock(8)"));
 	hwclock_exit(ctl, EXIT_SUCCESS);
 }
 

[PATCH 03/12] hwclock: update usage() FILE name

[J William Piggott]

Use the util-linux standard file name instead of 'usageto'.

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

diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index d2f8ad1..28655b0 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -1200,44 +1200,44 @@ static void out_version(void)
 }
 
 static void __attribute__((__noreturn__))
-usage(const struct hwclock_control *ctl, FILE *usageto)
+usage(const struct hwclock_control *ctl, FILE *out)
 {
-	fputs(USAGE_HEADER, usageto);
-	fputs(_(" hwclock [function] [option...]\n"), usageto);
+	fputs(USAGE_HEADER, out);
+	fputs(_(" hwclock [function] [option...]\n"), out);
 
-	fputs(USAGE_SEPARATOR, usageto);
-	fputs(_(" Query or set the hardware clock\n"), usageto);
+	fputs(USAGE_SEPARATOR, out);
+	fputs(_(" Query or set the hardware clock\n"), out);
 
-	fputs(USAGE_SEPARATOR, usageto);
-	fputs(_("Functions:\n"), usageto);
+	fputs(USAGE_SEPARATOR, out);
+	fputs(_("Functions:\n"), out);
 	fputs(_(" -r, --show           read hardware clock and print result\n"
 		"     --get            read hardware clock and print drift corrected result\n"
-		"     --set            set the RTC to the time given with --date\n"), usageto);
+		"     --set            set the RTC to the time given with --date\n"), out);
 	fputs(_(" -s, --hctosys        set the system time from the hardware clock\n"
 		" -w, --systohc        set the hardware clock from the current system time\n"
 		"     --systz          set the system time based on the current timezone\n"
 		"     --adjust         adjust the RTC to account for systematic drift since\n"
-		"                        the clock was last set or adjusted\n"), usageto);
+		"                        the clock was last set or adjusted\n"), out);
 #if defined(__linux__) && defined(__alpha__)
 	fputs(_("     --getepoch       print out the kernel's hardware clock epoch value\n"
 		"     --setepoch       set the kernel's hardware clock epoch value to the \n"
-		"                        value given with --epoch\n"), usageto);
+		"                        value given with --epoch\n"), out);
 #endif
-	fputs(_("     --predict        predict RTC reading at time given with --date\n"), usageto);
+	fputs(_("     --predict        predict RTC reading at time given with --date\n"), out);
 
-	fputs(USAGE_OPTIONS, usageto);
+	fputs(USAGE_OPTIONS, out);
 	fputs(_(" -u, --utc            the hardware clock is kept in UTC\n"
-		" -l, --localtime      the hardware clock is kept in local time\n"), usageto);
+		" -l, --localtime      the hardware clock is kept in local time\n"), out);
 #ifdef __linux__
-	fputs(_(" -f, --rtc <file>     special /dev/... file to use instead of default\n"), usageto);
+	fputs(_(" -f, --rtc <file>     special /dev/... file to use instead of default\n"), out);
 #endif
-	fprintf(usageto, _(
+	fprintf(out, _(
 		"     --directisa      access the ISA bus directly instead of %s\n"
 		"     --date <time>    specifies the time to which to set the hardware clock\n"), _PATH_RTC_DEV);
 #if defined(__linux__) && defined(__alpha__)
-	fputs(_("     --epoch <year>   specifies the hardware clock's epoch value\n"), usageto);
+	fputs(_("     --epoch <year>   specifies the hardware clock's epoch value\n"), out);
 #endif
-	fprintf(usageto, _(
+	fprintf(out, _(
 		"     --update-drift   update drift factor in %1$s (requires\n"
 		"                        --set or --systohc)\n"
 		"     --noadjfile      do not access %1$s; this requires the use of\n"
@@ -1245,11 +1245,11 @@ usage(const struct hwclock_control *ctl, FILE *usageto)
 		"     --adjfile <file> specifies the path to the adjust file;\n"
 		"                        the default is %1$s\n"), _PATH_ADJTIME);
 	fputs(_("     --test           do not update anything, just show what would happen\n"
-		" -D, --debug          debugging mode\n"), usageto);
-	fputs(USAGE_SEPARATOR, usageto);
-	fputs(USAGE_HELP, usageto);
-	fputs(USAGE_VERSION, usageto);
-	fprintf(usageto, USAGE_MAN_TAIL("hwclock(8)"));
+		" -D, --debug          debugging mode\n"), out);
+	fputs(USAGE_SEPARATOR, out);
+	fputs(USAGE_HELP, out);
+	fputs(USAGE_VERSION, out);
+	fprintf(out, USAGE_MAN_TAIL("hwclock(8)"));
 	hwclock_exit(ctl, EXIT_SUCCESS);
 }
 

[PATCH 04/12] hwclock: add usage() functions heading

[J William Piggott]

Make a functions heading, similar to the existing options heading.

* include/c.h: define USAGE_FUNCTIONS
* Documentation/boilerplate.c: add USAGE_FUNCTIONS
* sys-utils/hwclock.c add functions header to usage()

Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
 Documentation/boilerplate.c | 3 +++
 include/c.h                 | 3 ++-
 sys-utils/hwclock.c         | 3 +--
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/boilerplate.c b/Documentation/boilerplate.c
index 7da9374..057893b 100644
--- a/Documentation/boilerplate.c
+++ b/Documentation/boilerplate.c
@@ -33,6 +33,9 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
 {
 	fputs(USAGE_HEADER, out);
 	fprintf(out, _(" %s [options] file...\n"), program_invocation_short_name);
+	fputs(USAGE_FUNCTIONS, out);
+	fputs(_(" -s, --do-something      some specific task\n"), out);
+	fputs(_(" -o, --do-other          some different task\n"), out);
 	fputs(USAGE_OPTIONS, out);
 	fputs(_(" -n, --no-argument       option does not use argument\n"), out);
 	fputs(_("     --optional[=<arg>]  option argument is optional\n"), out);
diff --git a/include/c.h b/include/c.h
index a5162b9..9c19965 100644
--- a/include/c.h
+++ b/include/c.h
@@ -317,10 +317,11 @@ static inline int xusleep(useconds_t usec)
  */
 #define USAGE_HEADER     _("\nUsage:\n")
 #define USAGE_OPTIONS    _("\nOptions:\n")
+#define USAGE_FUNCTIONS  _("\nFunctions:\n")
 #define USAGE_SEPARATOR    "\n"
 #define USAGE_HELP       _(" -h, --help     display this help and exit\n")
 #define USAGE_VERSION    _(" -V, --version  output version information and exit\n")
-#define USAGE_MAN_TAIL(_man)   _("\nFor more details see %s.\n"), _man
+#define USAGE_MAN_TAIL(_man)   _("\nFor more details see %s\n"), _man
 
 #define UTIL_LINUX_VERSION _("%s from %s\n"), program_invocation_short_name, PACKAGE_STRING
 
diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 28655b0..9d6bb11 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -1208,8 +1208,7 @@ usage(const struct hwclock_control *ctl, FILE *out)
 	fputs(USAGE_SEPARATOR, out);
 	fputs(_(" Query or set the hardware clock\n"), out);
 
-	fputs(USAGE_SEPARATOR, out);
-	fputs(_("Functions:\n"), out);
+	fputs(USAGE_FUNCTIONS, out);
 	fputs(_(" -r, --show           read hardware clock and print result\n"
 		"     --get            read hardware clock and print drift corrected result\n"
 		"     --set            set the RTC to the time given with --date\n"), out);

[PATCH 05/12] include: update pathnames.h

[J William Piggott]

* use /dev/rtc0 (/dev/rtc was for the 'old' driver)
* remove hwclock Award workaround and alpha cmos paths
* relocate _PATH_BTMP from hwclock to login-utils
* add a comment for _PATH_BTMP and fix other login-utils comments
* add a comment for proc/cpuinfo
* remove empty shutdown.c comment from 4d43977f

Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
 include/pathnames.h | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/include/pathnames.h b/include/pathnames.h
index de6abe6..c07f9a6 100644
--- a/include/pathnames.h
+++ b/include/pathnames.h
@@ -57,12 +57,10 @@
 #define _PATH_TERMCOLORS_DIRNAME "terminal-colors.d"
 #define _PATH_TERMCOLORS_DIR	"/etc/" _PATH_TERMCOLORS_DIRNAME
 
-/* used in login-utils/shutdown.c */
-
-/* used in login-utils/setpwnam.h and login-utils/islocal.c */
+/* used in login-utils/{setpwnam.h,islocal.c} */
 #define _PATH_PASSWD		"/etc/passwd"
 
-/* used in login-utils/newgrp and login-utils/setpwnam.h*/
+/* used in login-utils/{newgrp.c,setpwnam.h} */
 #define _PATH_GSHADOW		"/etc/gshadow"
 
 /* used in login-utils/setpwnam.h */
@@ -70,6 +68,11 @@
 #define _PATH_SHADOW_PASSWD	"/etc/shadow"
 #define _PATH_SHELLS		"/etc/shells"
 
+/* used in login-utils/{login.c,su-common.c,last.c,lslogins.c}  */
+#ifndef _PATH_BTMP
+#define _PATH_BTMP		"/var/log/btmp"
+#endif
+
 /* used in term-utils/agetty.c */
 #define _PATH_ISSUE		"/etc/issue"
 #define _PATH_OS_RELEASE_ETC	"/etc/os-release"
@@ -154,15 +157,10 @@
 # define _PATH_ADJTIME		"/etc/adjtime"
 #endif
 
-#define _PATH_LASTDATE		"/var/lib/lastdate"
 #ifdef __ia64__
 # define _PATH_RTC_DEV		"/dev/efirtc"
 #else
-# define _PATH_RTC_DEV		"/dev/rtc"
-#endif
-
-#ifndef _PATH_BTMP
-#define _PATH_BTMP		"/var/log/btmp"
+# define _PATH_RTC_DEV		"/dev/rtc0"
 #endif
 
 /* raw paths*/
@@ -195,8 +193,7 @@
 /* ctrlaltdel paths */
 #define _PATH_PROC_CTRL_ALT_DEL	"/proc/sys/kernel/ctrl-alt-del"
 
-/* hwclock-cmos paths */
-#define _PATH_DEV_PORT		"/dev/port"
+/* lscpu paths */
 #define _PATH_PROC_CPUINFO	"/proc/cpuinfo"
 
 #endif /* PATHNAMES_H */

[PATCH 06/12] hwclock: use RTC in help output

[J William Piggott]

Switching between 'hardware clock' and 'RTC' is ambiguous.
RTC is used due to space constraints, so use it consistently.

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

diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 9d6bb11..25b159d 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -1206,35 +1206,35 @@ usage(const struct hwclock_control *ctl, FILE *out)
 	fputs(_(" hwclock [function] [option...]\n"), out);
 
 	fputs(USAGE_SEPARATOR, out);
-	fputs(_(" Query or set the hardware clock\n"), out);
+	fputs(_(" Query or set the RTC (Real Time Clock / Hardware Clock)\n"), out);
 
 	fputs(USAGE_FUNCTIONS, out);
-	fputs(_(" -r, --show           read hardware clock and print result\n"
-		"     --get            read hardware clock and print drift corrected result\n"
+	fputs(_(" -r, --show           read the RTC and print result\n"
+		"     --get            read the RTC and print drift corrected result\n"
 		"     --set            set the RTC to the time given with --date\n"), out);
-	fputs(_(" -s, --hctosys        set the system time from the hardware clock\n"
-		" -w, --systohc        set the hardware clock from the current system time\n"
+	fputs(_(" -s, --hctosys        set the system time from the RTC\n"
+		" -w, --systohc        set the RTC from the current system time\n"
 		"     --systz          set the system time based on the current timezone\n"
 		"     --adjust         adjust the RTC to account for systematic drift since\n"
 		"                        the clock was last set or adjusted\n"), out);
 #if defined(__linux__) && defined(__alpha__)
-	fputs(_("     --getepoch       print out the kernel's hardware clock epoch value\n"
-		"     --setepoch       set the kernel's hardware clock epoch value to the \n"
+	fputs(_("     --getepoch       print out the kernel's RTC epoch value\n"
+		"     --setepoch       set the kernel's RTC epoch value to the \n"
 		"                        value given with --epoch\n"), out);
 #endif
-	fputs(_("     --predict        predict RTC reading at time given with --date\n"), out);
+	fputs(_("     --predict        predict the RTC reading at time given with --date\n"), out);
 
 	fputs(USAGE_OPTIONS, out);
-	fputs(_(" -u, --utc            the hardware clock is kept in UTC\n"
-		" -l, --localtime      the hardware clock is kept in local time\n"), out);
+	fputs(_(" -u, --utc            the RTC is kept in UTC\n"
+		" -l, --localtime      the RTC is kept in local time\n"), out);
 #ifdef __linux__
 	fputs(_(" -f, --rtc <file>     special /dev/... file to use instead of default\n"), out);
 #endif
 	fprintf(out, _(
 		"     --directisa      access the ISA bus directly instead of %s\n"
-		"     --date <time>    specifies the time to which to set the hardware clock\n"), _PATH_RTC_DEV);
+		"     --date <time>    specifies the time to which to set the RTC\n"), _PATH_RTC_DEV);
 #if defined(__linux__) && defined(__alpha__)
-	fputs(_("     --epoch <year>   specifies the hardware clock's epoch value\n"), out);
+	fputs(_("     --epoch <year>   specifies the RTC's epoch value\n"), out);
 #endif
 	fprintf(out, _(
 		"     --update-drift   update drift factor in %1$s (requires\n"

[PATCH 07/12] hwclock: update --help content and grammar

[J William Piggott]

Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
 include/c.h         |  4 ++--
 sys-utils/hwclock.c | 48 +++++++++++++++++++++---------------------------
 2 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/include/c.h b/include/c.h
index 9c19965..bd073fc 100644
--- a/include/c.h
+++ b/include/c.h
@@ -319,8 +319,8 @@ static inline int xusleep(useconds_t usec)
 #define USAGE_OPTIONS    _("\nOptions:\n")
 #define USAGE_FUNCTIONS  _("\nFunctions:\n")
 #define USAGE_SEPARATOR    "\n"
-#define USAGE_HELP       _(" -h, --help     display this help and exit\n")
-#define USAGE_VERSION    _(" -V, --version  output version information and exit\n")
+#define USAGE_HELP       _(" -h, --help     display this information and exit\n")
+#define USAGE_VERSION    _(" -V, --version  display version information and exit\n")
 #define USAGE_MAN_TAIL(_man)   _("\nFor more details see %s\n"), _man
 
 #define UTIL_LINUX_VERSION _("%s from %s\n"), program_invocation_short_name, PACKAGE_STRING
diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 25b159d..33d1bd2 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -1209,42 +1209,36 @@ usage(const struct hwclock_control *ctl, FILE *out)
 	fputs(_(" Query or set the RTC (Real Time Clock / Hardware Clock)\n"), out);
 
 	fputs(USAGE_FUNCTIONS, out);
-	fputs(_(" -r, --show           read the RTC and print result\n"
-		"     --get            read the RTC and print drift corrected result\n"
-		"     --set            set the RTC to the time given with --date\n"), out);
+	fputs(_(" -r, --show           display the RTC time\n"
+		"     --get            display drift corrected RTC time\n"
+		"     --set            set the RTC according to --date\n"), out);
 	fputs(_(" -s, --hctosys        set the system time from the RTC\n"
-		" -w, --systohc        set the RTC from the current system time\n"
-		"     --systz          set the system time based on the current timezone\n"
-		"     --adjust         adjust the RTC to account for systematic drift since\n"
-		"                        the clock was last set or adjusted\n"), out);
+		" -w, --systohc        set the RTC from the system time\n"
+		"     --systz          send timescale configurations to the kernel\n"
+		"     --adjust         adjust the RTC to account for systematic drift\n"), out);
 #if defined(__linux__) && defined(__alpha__)
-	fputs(_("     --getepoch       print out the kernel's RTC epoch value\n"
-		"     --setepoch       set the kernel's RTC epoch value to the \n"
-		"                        value given with --epoch\n"), out);
+	fputs(_("     --getepoch       display the RTC epoch\n"
+		"     --setepoch       set the RTC epoch according to --epoch\n"), out);
 #endif
-	fputs(_("     --predict        predict the RTC reading at time given with --date\n"), out);
-
+	fputs(_("     --predict        predict the drifted RTC time according to --date\n"), out);
 	fputs(USAGE_OPTIONS, out);
-	fputs(_(" -u, --utc            the RTC is kept in UTC\n"
-		" -l, --localtime      the RTC is kept in local time\n"), out);
+	fputs(_(" -u, --utc            inform hwclock the RTC timescale is UTC\n"
+		" -l, --localtime      inform hwclock the RTC timescale is Local\n"), out);
+	fprintf(out, _(
 #ifdef __linux__
-	fputs(_(" -f, --rtc <file>     special /dev/... file to use instead of default\n"), out);
+		" -f, --rtc <file>     use an alternate file to %1$s\n"
 #endif
-	fprintf(out, _(
-		"     --directisa      access the ISA bus directly instead of %s\n"
-		"     --date <time>    specifies the time to which to set the RTC\n"), _PATH_RTC_DEV);
+		"     --directisa      use the ISA bus instead of %1$s access\n"
+		"     --date <time>    date/time input for --set and --predict\n"), _PATH_RTC_DEV);
 #if defined(__linux__) && defined(__alpha__)
-	fputs(_("     --epoch <year>   specifies the RTC's epoch value\n"), out);
+	fputs(_("     --epoch <year>   epoch input for --setepoch\n"), out);
 #endif
 	fprintf(out, _(
-		"     --update-drift   update drift factor in %1$s (requires\n"
-		"                        --set or --systohc)\n"
-		"     --noadjfile      do not access %1$s; this requires the use of\n"
-		"                        either --utc or --localtime\n"
-		"     --adjfile <file> specifies the path to the adjust file;\n"
-		"                        the default is %1$s\n"), _PATH_ADJTIME);
-	fputs(_("     --test           do not update anything, just show what would happen\n"
-		" -D, --debug          debugging mode\n"), out);
+		"     --update-drift   update drift factor (requires --set or --systohc)\n"
+		"     --noadjfile      do not use %1$s\n"
+		"     --adjfile <file> use an alternate file to %1$s\n"), _PATH_ADJTIME);
+	fputs(_("     --test           dry run; use -D to view what would have happened\n"
+		" -D, --debug          use debug mode\n"), out);
 	fputs(USAGE_SEPARATOR, out);
 	fputs(USAGE_HELP, out);
 	fputs(USAGE_VERSION, out);

[PATCH 08/12] hwclock: slice up the usage text

[J William Piggott]

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

diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 33d1bd2..5868ad6 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -1209,36 +1209,36 @@ usage(const struct hwclock_control *ctl, FILE *out)
 	fputs(_(" Query or set the RTC (Real Time Clock / Hardware Clock)\n"), out);
 
 	fputs(USAGE_FUNCTIONS, out);
-	fputs(_(" -r, --show           display the RTC time\n"
-		"     --get            display drift corrected RTC time\n"
-		"     --set            set the RTC according to --date\n"), out);
-	fputs(_(" -s, --hctosys        set the system time from the RTC\n"
-		" -w, --systohc        set the RTC from the system time\n"
-		"     --systz          send timescale configurations to the kernel\n"
-		"     --adjust         adjust the RTC to account for systematic drift\n"), out);
+	fputs(_(" -r, --show           display the RTC time\n"), out);
+	fputs(_("     --get            display drift corrected RTC time\n"), out);
+	fputs(_("     --set            set the RTC according to --date\n"), out);
+	fputs(_(" -s, --hctosys        set the system time from the RTC\n"), out);
+	fputs(_(" -w, --systohc        set the RTC from the system time\n"), out);
+	fputs(_("     --systz          send timescale configurations to the kernel\n"), out);
+	fputs(_("     --adjust         adjust the RTC to account for systematic drift\n"), out);
 #if defined(__linux__) && defined(__alpha__)
-	fputs(_("     --getepoch       display the RTC epoch\n"
-		"     --setepoch       set the RTC epoch according to --epoch\n"), out);
+	fputs(_("     --getepoch       display the RTC epoch\n"), out);
+	fputs(_("     --setepoch       set the RTC epoch according to --epoch\n"), out);
 #endif
 	fputs(_("     --predict        predict the drifted RTC time according to --date\n"), out);
 	fputs(USAGE_OPTIONS, out);
-	fputs(_(" -u, --utc            inform hwclock the RTC timescale is UTC\n"
-		" -l, --localtime      inform hwclock the RTC timescale is Local\n"), out);
+	fputs(_(" -u, --utc            inform hwclock the RTC timescale is UTC\n"), out);
+	fputs(_(" -l, --localtime      inform hwclock the RTC timescale is Local\n"), out);
 	fprintf(out, _(
 #ifdef __linux__
 		" -f, --rtc <file>     use an alternate file to %1$s\n"
 #endif
-		"     --directisa      use the ISA bus instead of %1$s access\n"
-		"     --date <time>    date/time input for --set and --predict\n"), _PATH_RTC_DEV);
+		"     --directisa      use the ISA bus instead of %1$s access\n"), _PATH_RTC_DEV);
+	fputs(_("     --date <time>    date/time input for --set and --predict\n"), out);
 #if defined(__linux__) && defined(__alpha__)
 	fputs(_("     --epoch <year>   epoch input for --setepoch\n"), out);
 #endif
+	fputs(_("     --update-drift   update drift factor (requires --set or --systohc)\n"), out);
 	fprintf(out, _(
-		"     --update-drift   update drift factor (requires --set or --systohc)\n"
 		"     --noadjfile      do not use %1$s\n"
 		"     --adjfile <file> use an alternate file to %1$s\n"), _PATH_ADJTIME);
-	fputs(_("     --test           dry run; use -D to view what would have happened\n"
-		" -D, --debug          use debug mode\n"), out);
+	fputs(_("     --test           dry run; use -D to view what would have happened\n"), out);
+	fputs(_(" -D, --debug          use debug mode\n"), out);
 	fputs(USAGE_SEPARATOR, out);
 	fputs(USAGE_HELP, out);
 	fputs(USAGE_VERSION, out);

[PATCH 09/12] hwclock: add --update-drift check

[J William Piggott]

Only allow --update-drift for --set or --systohc

Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
 sys-utils/hwclock.8.in | 3 +--
 sys-utils/hwclock.c    | 7 ++++++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/sys-utils/hwclock.8.in b/sys-utils/hwclock.8.in
index cf33e63..72b842e 100644
--- a/sys-utils/hwclock.8.in
+++ b/sys-utils/hwclock.8.in
@@ -376,9 +376,8 @@ in learning about the internal operations of hwclock.
 .B \-\-update\-drift
 Update the Hardware Clock's drift factor in
 .IR @ADJTIME_PATH@ .
-It is used with
+It can only be used with
 .BR \-\-set " or " \%\-\-systohc ,
-otherwise it is ignored.
 .sp
 A minimum four hour period between settings is required.  This is to
 avoid invalid calculations.  The longer the period, the more precise the
diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 5868ad6..7d69b7a 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -1233,7 +1233,7 @@ usage(const struct hwclock_control *ctl, FILE *out)
 #if defined(__linux__) && defined(__alpha__)
 	fputs(_("     --epoch <year>   epoch input for --setepoch\n"), out);
 #endif
-	fputs(_("     --update-drift   update drift factor (requires --set or --systohc)\n"), out);
+	fputs(_("     --update-drift   update the RTC drift factor\n"), out);
 	fprintf(out, _(
 		"     --noadjfile      do not use %1$s\n"
 		"     --adjfile <file> use an alternate file to %1$s\n"), _PATH_ADJTIME);
@@ -1465,6 +1465,11 @@ int main(int argc, char **argv)
 	if (!ctl.adj_file_name)
 		ctl.adj_file_name = _PATH_ADJTIME;
 
+	if (ctl.update && !ctl.set && !ctl.systohc) {
+		warnx(_("--update-drift requires --set or --systohc"));
+		hwclock_exit(&ctl, EX_USAGE);
+	}
+
 	if (ctl.noadjfile && !ctl.utc && !ctl.local_opt) {
 		warnx(_("With --noadjfile, you must specify "
 			"either --utc or --localtime"));

[PATCH 10/12] Docs: update howto-usage-function.txt

[J William Piggott]

Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
 Documentation/howto-usage-function.txt | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/Documentation/howto-usage-function.txt b/Documentation/howto-usage-function.txt
index 54d3084..8da8deb 100644
--- a/Documentation/howto-usage-function.txt
+++ b/Documentation/howto-usage-function.txt
@@ -10,8 +10,7 @@ other purpose:
 The rule of thumb with other options is that once they exist, you may
 not change them, nor change how they work, nor remove them.
 
-Notice that '-?' is not expected to be a synonym of '--help', but is an
-unknown option resulting in a usage print-out due to a getopt failure.
+See Legacy options below.
 
 
 How a usage text is supposed to look
@@ -118,6 +117,7 @@ entail for translators will pay off later, at the time of the next change,
 when they will not need to search in the long fuzzy text what was changed,
 where, how, and whether it was the only change.
 
+
 Synopsis
 --------
 
@@ -143,13 +143,15 @@ Some commands use peculiar options and arguments.  These will continue
 to be supported, but anything like them will not be accepted as new
 additions.  A short list of examples:
 
-- Other characters than '-' to start an option.  See '+' in 'more'.
-- Using a number as an option argument.  See '-<number>' in 'more'.
+- Characters other than '-' to start an option.  See '+' in 'more'.
+- Using a number as an option.  See '-<number>' in 'more'.
 - Long options that start with a single '-'.  See 'setterm'.
+- '-?' is not expected to be a synonym of '--help', but is an unknown
+  option resulting in a suggestion to try --help due to a getopt failure.
 
 
 Example file
 ------------
 
-The file disk-utils/delpart.c is a minimal example of how to write
+The file ./boilerplate.c is a minimal example of how to write
 a usage function, set up option parsing, version printing and so on.

[PATCH 11/12] hwclock: remove unused usage() FILE argument

[J William Piggott]

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

diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 7d69b7a..636d36a 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -1200,49 +1200,49 @@ static void out_version(void)
 }
 
 static void __attribute__((__noreturn__))
-usage(const struct hwclock_control *ctl, FILE *out)
+usage(const struct hwclock_control *ctl)
 {
-	fputs(USAGE_HEADER, out);
-	fputs(_(" hwclock [function] [option...]\n"), out);
-
-	fputs(USAGE_SEPARATOR, out);
-	fputs(_(" Query or set the RTC (Real Time Clock / Hardware Clock)\n"), out);
-
-	fputs(USAGE_FUNCTIONS, out);
-	fputs(_(" -r, --show           display the RTC time\n"), out);
-	fputs(_("     --get            display drift corrected RTC time\n"), out);
-	fputs(_("     --set            set the RTC according to --date\n"), out);
-	fputs(_(" -s, --hctosys        set the system time from the RTC\n"), out);
-	fputs(_(" -w, --systohc        set the RTC from the system time\n"), out);
-	fputs(_("     --systz          send timescale configurations to the kernel\n"), out);
-	fputs(_("     --adjust         adjust the RTC to account for systematic drift\n"), out);
+	fputs(USAGE_HEADER, stdout);
+	puts(_(" hwclock [function] [option...]"));
+
+	fputs(USAGE_SEPARATOR, stdout);
+	puts(_(" Query or set the RTC (Real Time Clock / Hardware Clock)"));
+
+	fputs(USAGE_FUNCTIONS, stdout);
+	puts(_(" -r, --show           display the RTC time"));
+	puts(_("     --get            display drift corrected RTC time"));
+	puts(_("     --set            set the RTC according to --date"));
+	puts(_(" -s, --hctosys        set the system time from the RTC"));
+	puts(_(" -w, --systohc        set the RTC from the system time"));
+	puts(_("     --systz          send timescale configurations to the kernel"));
+	puts(_("     --adjust         adjust the RTC to account for systematic drift"));
 #if defined(__linux__) && defined(__alpha__)
-	fputs(_("     --getepoch       display the RTC epoch\n"), out);
-	fputs(_("     --setepoch       set the RTC epoch according to --epoch\n"), out);
+	puts(_("     --getepoch       display the RTC epoch"));
+	puts(_("     --setepoch       set the RTC epoch according to --epoch"));
 #endif
-	fputs(_("     --predict        predict the drifted RTC time according to --date\n"), out);
-	fputs(USAGE_OPTIONS, out);
-	fputs(_(" -u, --utc            inform hwclock the RTC timescale is UTC\n"), out);
-	fputs(_(" -l, --localtime      inform hwclock the RTC timescale is Local\n"), out);
-	fprintf(out, _(
+	puts(_("     --predict        predict the drifted RTC time according to --date"));
+	fputs(USAGE_OPTIONS, stdout);
+	puts(_(" -u, --utc            inform hwclock the RTC timescale is UTC"));
+	puts(_(" -l, --localtime      inform hwclock the RTC timescale is Local"));
+	printf(_(
 #ifdef __linux__
-		" -f, --rtc <file>     use an alternate file to %1$s\n"
+	       " -f, --rtc <file>     use an alternate file to %1$s\n"
 #endif
-		"     --directisa      use the ISA bus instead of %1$s access\n"), _PATH_RTC_DEV);
-	fputs(_("     --date <time>    date/time input for --set and --predict\n"), out);
+	       "     --directisa      use the ISA bus instead of %1$s access\n"), _PATH_RTC_DEV);
+	puts(_("     --date <time>    date/time input for --set and --predict"));
 #if defined(__linux__) && defined(__alpha__)
-	fputs(_("     --epoch <year>   epoch input for --setepoch\n"), out);
+	puts(_("     --epoch <year>   epoch input for --setepoch"));
 #endif
-	fputs(_("     --update-drift   update the RTC drift factor\n"), out);
-	fprintf(out, _(
-		"     --noadjfile      do not use %1$s\n"
-		"     --adjfile <file> use an alternate file to %1$s\n"), _PATH_ADJTIME);
-	fputs(_("     --test           dry run; use -D to view what would have happened\n"), out);
-	fputs(_(" -D, --debug          use debug mode\n"), out);
-	fputs(USAGE_SEPARATOR, out);
-	fputs(USAGE_HELP, out);
-	fputs(USAGE_VERSION, out);
-	fprintf(out, USAGE_MAN_TAIL("hwclock(8)"));
+	puts(_("     --update-drift   update the RTC drift factor"));
+	printf(_(
+	       "     --noadjfile      do not use %1$s\n"
+	       "     --adjfile <file> use an alternate file to %1$s\n"), _PATH_ADJTIME);
+	puts(_("     --test           dry run; use -D to view what would have happened"));
+	puts(_(" -D, --debug          use debug mode"));
+	fputs(USAGE_SEPARATOR, stdout);
+	fputs(USAGE_HELP, stdout);
+	fputs(USAGE_VERSION, stdout);
+	fprintf(stdout, USAGE_MAN_TAIL("hwclock(8)"));
 	hwclock_exit(ctl, EXIT_SUCCESS);
 }
 
@@ -1448,7 +1448,7 @@ int main(int argc, char **argv)
 			out_version();
 			return 0;
 		case 'h':			/* --help */
-			usage(&ctl, stdout);
+			usage(&ctl);
 		default:
 			errtryhelp(EXIT_FAILURE);
 		}

[PATCH 12/12] hwclock: remove unused stdarg.h

[J William Piggott]

Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
 sys-utils/hwclock.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 636d36a..e05ee96 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -61,7 +61,6 @@
 #include <getopt.h>
 #include <limits.h>
 #include <math.h>
-#include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>

Re: [PATCH 04/12] hwclock: add usage() functions heading

[Karel Zak]

On Sun, Jun 18, 2017 at 08:41:45PM -0400, J William Piggott wrote:
> -#define USAGE_MAN_TAIL(_man)   _("\nFor more details see %s.\n"), _man
> +#define USAGE_MAN_TAIL(_man)   _("\nFor more details see %s\n"), _man

What's wrong with the period?

    Karel

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

Re: [PATCH 05/12] include: update pathnames.h

[Karel Zak]

On Sun, Jun 18, 2017 at 08:42:55PM -0400, J William Piggott wrote:
> 
> * use /dev/rtc0 (/dev/rtc was for the 'old' driver)
> * remove hwclock Award workaround and alpha cmos paths
> * relocate _PATH_BTMP from hwclock to login-utils
> * add a comment for _PATH_BTMP and fix other login-utils comments
> * add a comment for proc/cpuinfo
> * remove empty shutdown.c comment from 4d43977f
> 
> Signed-off-by: J William Piggott <elseifthen@gmx.com>
> ---
>  include/pathnames.h | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/include/pathnames.h b/include/pathnames.h
> index de6abe6..c07f9a6 100644
> --- a/include/pathnames.h
> +++ b/include/pathnames.h
> @@ -57,12 +57,10 @@
>  #define _PATH_TERMCOLORS_DIRNAME "terminal-colors.d"
>  #define _PATH_TERMCOLORS_DIR	"/etc/" _PATH_TERMCOLORS_DIRNAME
>  
> -/* used in login-utils/shutdown.c */
> -
> -/* used in login-utils/setpwnam.h and login-utils/islocal.c */
> +/* used in login-utils/{setpwnam.h,islocal.c} */
>  #define _PATH_PASSWD		"/etc/passwd"

Frankly, do we really need "used in ..." messages in this file? I
guess everyone is able to use "git grep" (etc). 

It's header file, it should be used everywhere, from this point of
view the "used in" messages seems strange at all. Maybe we can remove
these comments completely.

> -/* used in login-utils/newgrp and login-utils/setpwnam.h*/
> +/* used in login-utils/{newgrp.c,setpwnam.h} */
>  #define _PATH_GSHADOW		"/etc/gshadow"
>  
>  /* used in login-utils/setpwnam.h */
> @@ -70,6 +68,11 @@
>  #define _PATH_SHADOW_PASSWD	"/etc/shadow"
>  #define _PATH_SHELLS		"/etc/shells"
>  
> +/* used in login-utils/{login.c,su-common.c,last.c,lslogins.c}  */
> +#ifndef _PATH_BTMP
> +#define _PATH_BTMP		"/var/log/btmp"
> +#endif

 # define 
  ^
It's better to use space behind '#' within #ifdefs. 

    Karel

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

Re: [PATCH 07/12] hwclock: update --help content and grammar

[Karel Zak]

On Sun, Jun 18, 2017 at 08:45:09PM -0400, J William Piggott wrote:
> 
> Signed-off-by: J William Piggott <elseifthen@gmx.com>
> ---
>  include/c.h         |  4 ++--
>  sys-utils/hwclock.c | 48 +++++++++++++++++++++---------------------------
>  2 files changed, 23 insertions(+), 29 deletions(-)
> 
> diff --git a/include/c.h b/include/c.h
> index 9c19965..bd073fc 100644
> --- a/include/c.h
> +++ b/include/c.h
> @@ -319,8 +319,8 @@ static inline int xusleep(useconds_t usec)
>  #define USAGE_OPTIONS    _("\nOptions:\n")
>  #define USAGE_FUNCTIONS  _("\nFunctions:\n")
>  #define USAGE_SEPARATOR    "\n"
> -#define USAGE_HELP       _(" -h, --help     display this help and exit\n")
> -#define USAGE_VERSION    _(" -V, --version  output version information and exit\n")
> +#define USAGE_HELP       _(" -h, --help     display this information and exit\n")

 "display this help and exit\n"
or
 "display help information and exit\n"

sounds better than "this information".

> +#define USAGE_VERSION    _(" -V, --version  display version information and exit\n")

This is better than the original.

    Karel

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

Re: [PATCH 11/12] hwclock: remove unused usage() FILE argument

[Karel Zak]

On Sun, Jun 18, 2017 at 08:49:49PM -0400, J William Piggott wrote:
>  sys-utils/hwclock.c | 74 ++++++++++++++++++++++++++---------------------------
>  1 file changed, 37 insertions(+), 37 deletions(-)

It would be better to remove this patch from the pull-request; let's
keep fputs() in the code and wait for any solution from Rudi :-)

It's bad ideal to introduce any local (hwclock-only) solution right
now.

    Karel

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

Re: [PATCH 11/12] hwclock: remove unused usage() FILE argument

[J William Piggott]

On 06/20/2017 04:56 AM, Karel Zak wrote:
> On Sun, Jun 18, 2017 at 08:49:49PM -0400, J William Piggott wrote:
>>  sys-utils/hwclock.c | 74 ++++++++++++++++++++++++++---------------------------
>>  1 file changed, 37 insertions(+), 37 deletions(-)
> 
> It would be better to remove this patch from the pull-request; let's
> keep fputs() in the code and wait for any solution from Rudi :-)

Dropped.

> 
> It's bad ideal to introduce any local (hwclock-only) solution right
> now.
> 
>     Karel
> 

Re: [PATCH 07/12] hwclock: update --help content and grammar

[J William Piggott]

On 06/20/2017 04:51 AM, Karel Zak wrote:
> On Sun, Jun 18, 2017 at 08:45:09PM -0400, J William Piggott wrote:
>>
>> Signed-off-by: J William Piggott <elseifthen@gmx.com>
>> ---
>>  include/c.h         |  4 ++--
>>  sys-utils/hwclock.c | 48 +++++++++++++++++++++---------------------------
>>  2 files changed, 23 insertions(+), 29 deletions(-)
>>
>> diff --git a/include/c.h b/include/c.h
>> index 9c19965..bd073fc 100644
>> --- a/include/c.h
>> +++ b/include/c.h
>> @@ -319,8 +319,8 @@ static inline int xusleep(useconds_t usec)
>>  #define USAGE_OPTIONS    _("\nOptions:\n")
>>  #define USAGE_FUNCTIONS  _("\nFunctions:\n")
>>  #define USAGE_SEPARATOR    "\n"
>> -#define USAGE_HELP       _(" -h, --help     display this help and exit\n")
>> -#define USAGE_VERSION    _(" -V, --version  output version information and exit\n")
>> +#define USAGE_HELP       _(" -h, --help     display this information and exit\n")
> 
>  "display this help and exit\n"
> or
>  "display help information and exit\n"

Done.

> 
> sounds better than "this information".
> 
>> +#define USAGE_VERSION    _(" -V, --version  display version information and exit\n")
> 
> This is better than the original.
> 
>     Karel
> 

Re: [PATCH 05/12] include: update pathnames.h

[J William Piggott]

On 06/20/2017 04:44 AM, Karel Zak wrote:
> On Sun, Jun 18, 2017 at 08:42:55PM -0400, J William Piggott wrote:
>>
>> * use /dev/rtc0 (/dev/rtc was for the 'old' driver)
>> * remove hwclock Award workaround and alpha cmos paths
>> * relocate _PATH_BTMP from hwclock to login-utils
>> * add a comment for _PATH_BTMP and fix other login-utils comments
>> * add a comment for proc/cpuinfo
>> * remove empty shutdown.c comment from 4d43977f
>>
>> Signed-off-by: J William Piggott <elseifthen@gmx.com>
>> ---
>>  include/pathnames.h | 21 +++++++++------------
>>  1 file changed, 9 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/pathnames.h b/include/pathnames.h
>> index de6abe6..c07f9a6 100644
>> --- a/include/pathnames.h
>> +++ b/include/pathnames.h
>> @@ -57,12 +57,10 @@
>>  #define _PATH_TERMCOLORS_DIRNAME "terminal-colors.d"
>>  #define _PATH_TERMCOLORS_DIR	"/etc/" _PATH_TERMCOLORS_DIRNAME
>>  
>> -/* used in login-utils/shutdown.c */
>> -
>> -/* used in login-utils/setpwnam.h and login-utils/islocal.c */
>> +/* used in login-utils/{setpwnam.h,islocal.c} */
>>  #define _PATH_PASSWD		"/etc/passwd"
> 
> Frankly, do we really need "used in ..." messages in this file? I
> guess everyone is able to use "git grep" (etc). 
> 
> It's header file, it should be used everywhere, from this point of
> view the "used in" messages seems strange at all. Maybe we can remove
> these comments completely.

Done

> 
>> -/* used in login-utils/newgrp and login-utils/setpwnam.h*/
>> +/* used in login-utils/{newgrp.c,setpwnam.h} */
>>  #define _PATH_GSHADOW		"/etc/gshadow"
>>  
>>  /* used in login-utils/setpwnam.h */
>> @@ -70,6 +68,11 @@
>>  #define _PATH_SHADOW_PASSWD	"/etc/shadow"
>>  #define _PATH_SHELLS		"/etc/shells"
>>  
>> +/* used in login-utils/{login.c,su-common.c,last.c,lslogins.c}  */
>> +#ifndef _PATH_BTMP
>> +#define _PATH_BTMP		"/var/log/btmp"
>> +#endif
> 
>  # define 
>   ^
> It's better to use space behind '#' within #ifdefs. 

There were several this way. All changed.

> 
>     Karel
> 

Re: [PATCH 04/12] hwclock: add usage() functions heading

[J William Piggott]

On 06/20/2017 04:36 AM, Karel Zak wrote:
> On Sun, Jun 18, 2017 at 08:41:45PM -0400, J William Piggott wrote:
>> -#define USAGE_MAN_TAIL(_man)   _("\nFor more details see %s.\n"), _man
>> +#define USAGE_MAN_TAIL(_man)   _("\nFor more details see %s\n"), _man
> 
> What's wrong with the period?

_I put it back_, but here is my opinion why it shouldn't be there:

* the rest of usage() doesn't use line-ending periods

* the super class grammar rule is 'be consistent'

* usage() is a vertical list so line-ending periods are not required

* the \n at the beginning and end of this string means it is standalone
   sentence (vertical list) so a line-ending period is not required.

* the main purpose (perhaps the only purpose) of a line-ending period is
  to improve readability in paragraphs of text. That is, when one
  sentence ends and another begins in the middle of a line.

* a passage of text with mixed use of line-ending punctuation for
  sentences degrades readability. Long term readers are conditioned to
  recognize properly delimited text and will parse it with less
  scrutiny; only to find out part way through that the burden of
  separating sentences is (sometimes) on them.

* it just looks wrong to have a screen full of usage() text without a
  period in sight, only to have one show up at EOF.

Pretty much the same reasons that I advocate that message strings should
not use periods. They are vertical lists and do not require it; we have
some message strings that cannot have line-ending periods, so to follow
the 'be consistent' grammar rule, none of them should. An added benefit
would be that simple rules are easy document, easy to remember, and easy
to enforce. Easier than: with warn() don't use. Otherwise use. Except if
it is not a sentence. Well, maybe use it on fragments that the author
intended to be a complete sentence ... a simple 'don't use them' is much
easier, yes? What to do if a message string has two sentences on one
line? Put them on separate lines, otherwise rewrite them as a single
sentence.


> 
>     Karel
> 

Re: [PATCH 11/12] hwclock: remove unused usage() FILE argument

[Karel Zak]

On Sun, Jun 18, 2017 at 08:49:49PM -0400, J William Piggott wrote:
> 
> Signed-off-by: J William Piggott <elseifthen@gmx.com>
> ---
>  sys-utils/hwclock.c | 74 ++++++++++++++++++++++++++---------------------------
>  1 file changed, 37 insertions(+), 37 deletions(-)
> 
> diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
> index 7d69b7a..636d36a 100644
> --- a/sys-utils/hwclock.c
> +++ b/sys-utils/hwclock.c
> @@ -1200,49 +1200,49 @@ static void out_version(void)
>  }
>  
>  static void __attribute__((__noreturn__))
> -usage(const struct hwclock_control *ctl, FILE *out)
> +usage(const struct hwclock_control *ctl)
>  {
> -	fputs(USAGE_HEADER, out);
> -	fputs(_(" hwclock [function] [option...]\n"), out);
> -
> -	fputs(USAGE_SEPARATOR, out);
> -	fputs(_(" Query or set the RTC (Real Time Clock / Hardware Clock)\n"), out);
> -
> -	fputs(USAGE_FUNCTIONS, out);
> -	fputs(_(" -r, --show           display the RTC time\n"), out);
> -	fputs(_("     --get            display drift corrected RTC time\n"), out);
> -	fputs(_("     --set            set the RTC according to --date\n"), out);
> -	fputs(_(" -s, --hctosys        set the system time from the RTC\n"), out);
> -	fputs(_(" -w, --systohc        set the RTC from the system time\n"), out);
> -	fputs(_("     --systz          send timescale configurations to the kernel\n"), out);
> -	fputs(_("     --adjust         adjust the RTC to account for systematic drift\n"), out);
> +	fputs(USAGE_HEADER, stdout);
> +	puts(_(" hwclock [function] [option...]"));
> +
> +	fputs(USAGE_SEPARATOR, stdout);
> +	puts(_(" Query or set the RTC (Real Time Clock / Hardware Clock)"));
            ^
We don't use this space here. Fixed.

    Karel

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

Re: [PATCH 00/12] pull request

[Karel Zak]

On Sun, Jun 18, 2017 at 08:35:25PM -0400, J William Piggott wrote:
> are available in the git repository at:
> 
>   git@github.com:jwpi/util-linux.git 170427

Merged. Thanks!

    Karel

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

Re: [PATCH 04/12] hwclock: add usage() functions heading

[Ruediger Meier]

On Monday 19 June 2017, J William Piggott wrote:
> Make a functions heading, similar to the existing options heading.
>
> * include/c.h: define USAGE_FUNCTIONS
> * Documentation/boilerplate.c: add USAGE_FUNCTIONS
> * sys-utils/hwclock.c add functions header to usage()
>
> Signed-off-by: J William Piggott <elseifthen@gmx.com>
> ---
>  Documentation/boilerplate.c | 3 +++
>  include/c.h                 | 3 ++-
>  sys-utils/hwclock.c         | 3 +--
>  3 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/boilerplate.c
> b/Documentation/boilerplate.c index 7da9374..057893b 100644
> --- a/Documentation/boilerplate.c
> +++ b/Documentation/boilerplate.c
> @@ -33,6 +33,9 @@ static void __attribute__((__noreturn__))
> usage(FILE *out) {
>  	fputs(USAGE_HEADER, out);
>  	fprintf(out, _(" %s [options] file...\n"),
> program_invocation_short_name); +	fputs(USAGE_FUNCTIONS, out);
> +	fputs(_(" -s, --do-something      some specific task\n"), out);
> +	fputs(_(" -o, --do-other          some different task\n"), out);

These options -s and -o are not implemented in the boilerplate.c. Also 
the short usage line does not mention "functions" but "options" only.

I think this USAGE_FUNCTIONS looks quite good in hwclock and would be 
nice for some other commands too. But I guess 90% of all commands do 
not need this. So maybe we should not add this to the boilerplate.

cu,
Rudi

>  	fputs(USAGE_OPTIONS, out);
>  	fputs(_(" -n, --no-argument       option does not use argument\n"),
> out); fputs(_("     --optional[=<arg>]  option argument is
> optional\n"), out); diff --git a/include/c.h b/include/c.h
> index a5162b9..9c19965 100644
> --- a/include/c.h
> +++ b/include/c.h
> @@ -317,10 +317,11 @@ static inline int xusleep(useconds_t usec)
>   */
>  #define USAGE_HEADER     _("\nUsage:\n")
>  #define USAGE_OPTIONS    _("\nOptions:\n")
> +#define USAGE_FUNCTIONS  _("\nFunctions:\n")
>  #define USAGE_SEPARATOR    "\n"
>  #define USAGE_HELP       _(" -h, --help     display this help and
> exit\n") #define USAGE_VERSION    _(" -V, --version  output version
> information and exit\n") -#define USAGE_MAN_TAIL(_man)   _("\nFor
> more details see %s.\n"), _man +#define USAGE_MAN_TAIL(_man)  
> _("\nFor more details see %s\n"), _man
>
>  #define UTIL_LINUX_VERSION _("%s from %s\n"),
> program_invocation_short_name, PACKAGE_STRING
>
> diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
> index 28655b0..9d6bb11 100644
> --- a/sys-utils/hwclock.c
> +++ b/sys-utils/hwclock.c
> @@ -1208,8 +1208,7 @@ usage(const struct hwclock_control *ctl, FILE
> *out) fputs(USAGE_SEPARATOR, out);
>  	fputs(_(" Query or set the hardware clock\n"), out);
>
> -	fputs(USAGE_SEPARATOR, out);
> -	fputs(_("Functions:\n"), out);
> +	fputs(USAGE_FUNCTIONS, out);
>  	fputs(_(" -r, --show           read hardware clock and print
> result\n" "     --get            read hardware clock and print drift
> corrected result\n" "     --set            set the RTC to the time
> given with --date\n"), out); --
> To unsubscribe from this list: send the line "unsubscribe util-linux"
> in the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/12] hwclock: update --help content and grammar

[Ruediger Meier]

On Tuesday 20 June 2017, Karel Zak wrote:
> On Sun, Jun 18, 2017 at 08:45:09PM -0400, J William Piggott wrote:
> > Signed-off-by: J William Piggott <elseifthen@gmx.com>
> > ---
> >  include/c.h         |  4 ++--
> >  sys-utils/hwclock.c | 48
> > +++++++++++++++++++++--------------------------- 2 files changed,
> > 23 insertions(+), 29 deletions(-)
> >
> > diff --git a/include/c.h b/include/c.h
> > index 9c19965..bd073fc 100644
> > --- a/include/c.h
> > +++ b/include/c.h
> > @@ -319,8 +319,8 @@ static inline int xusleep(useconds_t usec)
> >  #define USAGE_OPTIONS    _("\nOptions:\n")
> >  #define USAGE_FUNCTIONS  _("\nFunctions:\n")
> >  #define USAGE_SEPARATOR    "\n"
> > -#define USAGE_HELP       _(" -h, --help     display this help and
> > exit\n") -#define USAGE_VERSION    _(" -V, --version  output
> > version information and exit\n") +#define USAGE_HELP       _(" -h,
> > --help     display this information and exit\n")
>
>  "display this help and exit\n"
> or
>  "display help information and exit\n"
>
> sounds better than "this information".

IMO "this" is important because it makes clear that running --help would 
not give you any better information than the one you see already.

Compare this:
$ xz --help | grep help
  -h, --help        display this short help and exit
  -H, --long-help   display the long help (lists also the ...

$ xz --long-help | grep help
  -h, --help        display the short help (lists only the basic ...
  -H, --long-help   display this long help and exit


Our old errtryhelp, USAGE_HELP and USAGE_VERSION were 100% the same like 
coreutils. This was a nice consensus and even a bit shorter.

Anyways, instead of improving the grammer and correctness I would rather 
make these trivial option descriptions as short as possible, like

 -h, --help     display this help
 -V, --version  show version

Everybody knows what these options do when they exist.


> > +#define USAGE_VERSION    _(" -V, --version  display version
> > information and exit\n")
>
> This is better than the original.
>
>     Karel

Re: [PATCH 07/12] hwclock: update --help content and grammar

[Karel Zak]

On Wed, Jun 21, 2017 at 03:05:04PM +0200, Ruediger Meier wrote:
> On Tuesday 20 June 2017, Karel Zak wrote:
> > On Sun, Jun 18, 2017 at 08:45:09PM -0400, J William Piggott wrote:
> > > Signed-off-by: J William Piggott <elseifthen@gmx.com>
> > > ---
> > >  include/c.h         |  4 ++--
> > >  sys-utils/hwclock.c | 48
> > > +++++++++++++++++++++--------------------------- 2 files changed,
> > > 23 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/include/c.h b/include/c.h
> > > index 9c19965..bd073fc 100644
> > > --- a/include/c.h
> > > +++ b/include/c.h
> > > @@ -319,8 +319,8 @@ static inline int xusleep(useconds_t usec)
> > >  #define USAGE_OPTIONS    _("\nOptions:\n")
> > >  #define USAGE_FUNCTIONS  _("\nFunctions:\n")
> > >  #define USAGE_SEPARATOR    "\n"
> > > -#define USAGE_HELP       _(" -h, --help     display this help and
> > > exit\n") -#define USAGE_VERSION    _(" -V, --version  output
> > > version information and exit\n") +#define USAGE_HELP       _(" -h,
> > > --help     display this information and exit\n")
> >
> >  "display this help and exit\n"
> > or
> >  "display help information and exit\n"
> >
> > sounds better than "this information".
> 
> IMO "this" is important because it makes clear that running --help would 
> not give you any better information than the one you see already.
> 
> Compare this:
> $ xz --help | grep help
>   -h, --help        display this short help and exit
>   -H, --long-help   display the long help (lists also the ...
> 
> $ xz --long-help | grep help
>   -h, --help        display the short help (lists only the basic ...
>   -H, --long-help   display this long help and exit
> 
> 
> Our old errtryhelp, USAGE_HELP and USAGE_VERSION were 100% the same like 
> coreutils. This was a nice consensus and even a bit shorter.
> 
> Anyways, instead of improving the grammer and correctness I would rather 
> make these trivial option descriptions as short as possible, like
> 
>  -h, --help     display this help
>  -V, --version  show version
> 
> Everybody knows what these options do when they exist.

Good points, please, send a patch. It would be also nice to add any
comment to the c.h to avoid future updates on this area.

    Karel


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

Re: [PATCH 07/12] hwclock: update --help content and grammar

[Karel Zak]

On Tue, Jun 20, 2017 at 10:51:43AM +0200, Karel Zak wrote:
> On Sun, Jun 18, 2017 at 08:45:09PM -0400, J William Piggott wrote:
> > 
> > Signed-off-by: J William Piggott <elseifthen@gmx.com>
> > ---
> >  include/c.h         |  4 ++--
> >  sys-utils/hwclock.c | 48 +++++++++++++++++++++---------------------------
> >  2 files changed, 23 insertions(+), 29 deletions(-)
> > 
> > diff --git a/include/c.h b/include/c.h
> > index 9c19965..bd073fc 100644
> > --- a/include/c.h
> > +++ b/include/c.h
> > @@ -319,8 +319,8 @@ static inline int xusleep(useconds_t usec)
> >  #define USAGE_OPTIONS    _("\nOptions:\n")
> >  #define USAGE_FUNCTIONS  _("\nFunctions:\n")
> >  #define USAGE_SEPARATOR    "\n"
> > -#define USAGE_HELP       _(" -h, --help     display this help and exit\n")
> > -#define USAGE_VERSION    _(" -V, --version  output version information and exit\n")
> > +#define USAGE_HELP       _(" -h, --help     display this information and exit\n")

We can also add 

    #define USAGE_COLUMNS   _("Available columns:\n")

for -o,--output <columns>, and 

    #define USAGE_COMMANDS  _("\nCommands:\n")

    Karel


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

Re: [PATCH 07/12] hwclock: update --help content and grammar

[J William Piggott]

On 06/21/2017 10:55 AM, Karel Zak wrote:
> On Wed, Jun 21, 2017 at 03:05:04PM +0200, Ruediger Meier wrote:
>> On Tuesday 20 June 2017, Karel Zak wrote:
>>> On Sun, Jun 18, 2017 at 08:45:09PM -0400, J William Piggott wrote:
>>>> Signed-off-by: J William Piggott <elseifthen@gmx.com>
>>>> ---
>>>>  include/c.h         |  4 ++--
>>>>  sys-utils/hwclock.c | 48
>>>> +++++++++++++++++++++--------------------------- 2 files changed,
>>>> 23 insertions(+), 29 deletions(-)
>>>>
>>>> diff --git a/include/c.h b/include/c.h
>>>> index 9c19965..bd073fc 100644
>>>> --- a/include/c.h
>>>> +++ b/include/c.h
>>>> @@ -319,8 +319,8 @@ static inline int xusleep(useconds_t usec)
>>>>  #define USAGE_OPTIONS    _("\nOptions:\n")
>>>>  #define USAGE_FUNCTIONS  _("\nFunctions:\n")
>>>>  #define USAGE_SEPARATOR    "\n"
>>>> -#define USAGE_HELP       _(" -h, --help     display this help and
>>>> exit\n") -#define USAGE_VERSION    _(" -V, --version  output
>>>> version information and exit\n") +#define USAGE_HELP       _(" -h,
>>>> --help     display this information and exit\n")
>>>
>>>  "display this help and exit\n"
>>> or
>>>  "display help information and exit\n"
>>>
>>> sounds better than "this information".
>>
>> IMO "this" is important because it makes clear that running --help would 
>> not give you any better information than the one you see already.
>>
>> Compare this:
>> $ xz --help | grep help
>>   -h, --help        display this short help and exit
>>   -H, --long-help   display the long help (lists also the ...
>>
>> $ xz --long-help | grep help
>>   -h, --help        display the short help (lists only the basic ...
>>   -H, --long-help   display this long help and exit
>>
>>
>> Our old errtryhelp, USAGE_HELP and USAGE_VERSION were 100% the same like 
>> coreutils. This was a nice consensus and even a bit shorter.
>>
>> Anyways, instead of improving the grammer and correctness I would rather 
>> make these trivial option descriptions as short as possible, like
>>

My intent was to have these two very similar messages using the same
language. I think it would be easier to understand for non-native
speakers and easier for translators to work with if they did. I'm all
for making them more concise, but how about this:

  -h, --help     display this help
  -V, --version  display version
or
  -h, --help     show this help
  -V, --version  show version

I like display better.

>>  -h, --help     display this help
>>  -V, --version  show version
>>
>> Everybody knows what these options do when they exist.
> 
> Good points, please, send a patch. It would be also nice to add any
> comment to the c.h to avoid future updates on this area.
> 
>     Karel
> 
> 

Re: [PATCH 11/12] hwclock: remove unused usage() FILE argument

[J William Piggott]

On 06/21/2017 05:26 AM, Karel Zak wrote:
> On Sun, Jun 18, 2017 at 08:49:49PM -0400, J William Piggott wrote:
>>
>> Signed-off-by: J William Piggott <elseifthen@gmx.com>
>> ---
>>  sys-utils/hwclock.c | 74 ++++++++++++++++++++++++++---------------------------
>>  1 file changed, 37 insertions(+), 37 deletions(-)
>>
>> diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
>> index 7d69b7a..636d36a 100644
>> --- a/sys-utils/hwclock.c
>> +++ b/sys-utils/hwclock.c
>> @@ -1200,49 +1200,49 @@ static void out_version(void)
>>  }
>>  
>>  static void __attribute__((__noreturn__))
>> -usage(const struct hwclock_control *ctl, FILE *out)
>> +usage(const struct hwclock_control *ctl)
>>  {
>> -	fputs(USAGE_HEADER, out);
>> -	fputs(_(" hwclock [function] [option...]\n"), out);
>> -
>> -	fputs(USAGE_SEPARATOR, out);
>> -	fputs(_(" Query or set the RTC (Real Time Clock / Hardware Clock)\n"), out);
>> -
>> -	fputs(USAGE_FUNCTIONS, out);
>> -	fputs(_(" -r, --show           display the RTC time\n"), out);
>> -	fputs(_("     --get            display drift corrected RTC time\n"), out);
>> -	fputs(_("     --set            set the RTC according to --date\n"), out);
>> -	fputs(_(" -s, --hctosys        set the system time from the RTC\n"), out);
>> -	fputs(_(" -w, --systohc        set the RTC from the system time\n"), out);
>> -	fputs(_("     --systz          send timescale configurations to the kernel\n"), out);
>> -	fputs(_("     --adjust         adjust the RTC to account for systematic drift\n"), out);
>> +	fputs(USAGE_HEADER, stdout);
>> +	puts(_(" hwclock [function] [option...]"));
>> +
>> +	fputs(USAGE_SEPARATOR, stdout);
>> +	puts(_(" Query or set the RTC (Real Time Clock / Hardware Clock)"));
>             ^
> We don't use this space here. Fixed.

Ah, I miss read this in howto-usage-function.txt:

> The usage output begins with an empty line, followed by 'Usage:', and
> then the synopsis on the line after that.  The synopsis and option-
> description lines are all indented by one space (0x40).

I read 'synopsis and description', not 'option-description'. Also, it's
the entire option line, not the option-description that is indented. 

There is not an example in boilerplate.c for a command description,
there probably should be.

> 
>     Karel
> 

Re: [PATCH 04/12] hwclock: add usage() functions heading

[J William Piggott]

On 06/21/2017 08:21 AM, Ruediger Meier wrote:
> On Monday 19 June 2017, J William Piggott wrote:
>> Make a functions heading, similar to the existing options heading.
>>
>> * include/c.h: define USAGE_FUNCTIONS
>> * Documentation/boilerplate.c: add USAGE_FUNCTIONS
>> * sys-utils/hwclock.c add functions header to usage()
>>
>> Signed-off-by: J William Piggott <elseifthen@gmx.com>
>> ---
>>  Documentation/boilerplate.c | 3 +++
>>  include/c.h                 | 3 ++-
>>  sys-utils/hwclock.c         | 3 +--
>>  3 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/boilerplate.c
>> b/Documentation/boilerplate.c index 7da9374..057893b 100644
>> --- a/Documentation/boilerplate.c
>> +++ b/Documentation/boilerplate.c
>> @@ -33,6 +33,9 @@ static void __attribute__((__noreturn__))
>> usage(FILE *out) {
>>  	fputs(USAGE_HEADER, out);
>>  	fprintf(out, _(" %s [options] file...\n"),
>> program_invocation_short_name); +	fputs(USAGE_FUNCTIONS, out);
>> +	fputs(_(" -s, --do-something      some specific task\n"), out);
>> +	fputs(_(" -o, --do-other          some different task\n"), out);
> 
> These options -s and -o are not implemented in the boilerplate.c. Also 
> the short usage line does not mention "functions" but "options" only.

Good point, I didn't think to add them.

> 
> I think this USAGE_FUNCTIONS looks quite good in hwclock and would be 
> nice for some other commands too. But I guess 90% of all commands do 
> not need this. So maybe we should not add this to the boilerplate.

I agree that it is not common, but I think it belongs in boilerplate for
discovery purposes by new contributors.

> 
> cu,
> Rudi
> 
>>  	fputs(USAGE_OPTIONS, out);
>>  	fputs(_(" -n, --no-argument       option does not use argument\n"),
>> out); fputs(_("     --optional[=<arg>]  option argument is
>> optional\n"), out); diff --git a/include/c.h b/include/c.h
>> index a5162b9..9c19965 100644
>> --- a/include/c.h
>> +++ b/include/c.h
>> @@ -317,10 +317,11 @@ static inline int xusleep(useconds_t usec)
>>   */
>>  #define USAGE_HEADER     _("\nUsage:\n")
>>  #define USAGE_OPTIONS    _("\nOptions:\n")
>> +#define USAGE_FUNCTIONS  _("\nFunctions:\n")
>>  #define USAGE_SEPARATOR    "\n"
>>  #define USAGE_HELP       _(" -h, --help     display this help and
>> exit\n") #define USAGE_VERSION    _(" -V, --version  output version
>> information and exit\n") -#define USAGE_MAN_TAIL(_man)   _("\nFor
>> more details see %s.\n"), _man +#define USAGE_MAN_TAIL(_man)  
>> _("\nFor more details see %s\n"), _man
>>
>>  #define UTIL_LINUX_VERSION _("%s from %s\n"),
>> program_invocation_short_name, PACKAGE_STRING
>>
>> diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
>> index 28655b0..9d6bb11 100644
>> --- a/sys-utils/hwclock.c
>> +++ b/sys-utils/hwclock.c
>> @@ -1208,8 +1208,7 @@ usage(const struct hwclock_control *ctl, FILE
>> *out) fputs(USAGE_SEPARATOR, out);
>>  	fputs(_(" Query or set the hardware clock\n"), out);
>>
>> -	fputs(USAGE_SEPARATOR, out);
>> -	fputs(_("Functions:\n"), out);
>> +	fputs(USAGE_FUNCTIONS, out);
>>  	fputs(_(" -r, --show           read hardware clock and print
>> result\n" "     --get            read hardware clock and print drift
>> corrected result\n" "     --set            set the RTC to the time
>> given with --date\n"), out); --
>> To unsubscribe from this list: send the line "unsubscribe util-linux"
>> in the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 07/12] hwclock: update --help content and grammar

[J William Piggott]

On 06/21/2017 11:01 AM, Karel Zak wrote:
> On Tue, Jun 20, 2017 at 10:51:43AM +0200, Karel Zak wrote:
>> On Sun, Jun 18, 2017 at 08:45:09PM -0400, J William Piggott wrote:
>>>
>>> Signed-off-by: J William Piggott <elseifthen@gmx.com>
>>> ---
>>>  include/c.h         |  4 ++--
>>>  sys-utils/hwclock.c | 48 +++++++++++++++++++++---------------------------
>>>  2 files changed, 23 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/include/c.h b/include/c.h
>>> index 9c19965..bd073fc 100644
>>> --- a/include/c.h
>>> +++ b/include/c.h
>>> @@ -319,8 +319,8 @@ static inline int xusleep(useconds_t usec)
>>>  #define USAGE_OPTIONS    _("\nOptions:\n")
>>>  #define USAGE_FUNCTIONS  _("\nFunctions:\n")
>>>  #define USAGE_SEPARATOR    "\n"
>>> -#define USAGE_HELP       _(" -h, --help     display this help and exit\n")
>>> -#define USAGE_VERSION    _(" -V, --version  output version information and exit\n")
>>> +#define USAGE_HELP       _(" -h, --help     display this information and exit\n")
> 
> We can also add 
> 
>     #define USAGE_COLUMNS   _("Available columns:\n")
> 
> for -o,--output <columns>, and 
> 
>     #define USAGE_COMMANDS  _("\nCommands:\n")

Okay

> 
>     Karel
> 
> 

Re: [PATCH 07/12] hwclock: update --help content and grammar

[Ruediger Meier]

On Wednesday 21 June 2017, J William Piggott wrote:
> On 06/21/2017 10:55 AM, Karel Zak wrote:
> > On Wed, Jun 21, 2017 at 03:05:04PM +0200, Ruediger Meier wrote:
> >> On Tuesday 20 June 2017, Karel Zak wrote:
> >>> On Sun, Jun 18, 2017 at 08:45:09PM -0400, J William Piggott wrote:
> >>>> Signed-off-by: J William Piggott <elseifthen@gmx.com>
> >>>> ---
> >>>>  include/c.h         |  4 ++--
> >>>>  sys-utils/hwclock.c | 48
> >>>> +++++++++++++++++++++--------------------------- 2 files
> >>>> changed, 23 insertions(+), 29 deletions(-)
> >>>>
> >>>> diff --git a/include/c.h b/include/c.h
> >>>> index 9c19965..bd073fc 100644
> >>>> --- a/include/c.h
> >>>> +++ b/include/c.h
> >>>> @@ -319,8 +319,8 @@ static inline int xusleep(useconds_t usec)
> >>>>  #define USAGE_OPTIONS    _("\nOptions:\n")
> >>>>  #define USAGE_FUNCTIONS  _("\nFunctions:\n")
> >>>>  #define USAGE_SEPARATOR    "\n"
> >>>> -#define USAGE_HELP       _(" -h, --help     display this help
> >>>> and exit\n") -#define USAGE_VERSION    _(" -V, --version  output
> >>>> version information and exit\n") +#define USAGE_HELP       _("
> >>>> -h, --help     display this information and exit\n")
> >>>
> >>>  "display this help and exit\n"
> >>> or
> >>>  "display help information and exit\n"
> >>>
> >>> sounds better than "this information".
> >>
> >> IMO "this" is important because it makes clear that running --help
> >> would not give you any better information than the one you see
> >> already.
> >>
> >> Compare this:
> >> $ xz --help | grep help
> >>   -h, --help        display this short help and exit
> >>   -H, --long-help   display the long help (lists also the ...
> >>
> >> $ xz --long-help | grep help
> >>   -h, --help        display the short help (lists only the basic
> >> ... -H, --long-help   display this long help and exit
> >>
> >>
> >> Our old errtryhelp, USAGE_HELP and USAGE_VERSION were 100% the
> >> same like coreutils. This was a nice consensus and even a bit
> >> shorter.
> >>
> >> Anyways, instead of improving the grammer and correctness I would
> >> rather make these trivial option descriptions as short as
> >> possible, like
>
> My intent was to have these two very similar messages using the same
> language. I think it would be easier to understand for non-native
> speakers and easier for translators to work with if they did. I'm all
> for making them more concise, but how about this:
>
>   -h, --help     display this help
>   -V, --version  display version
> or
>   -h, --help     show this help
>   -V, --version  show version
>
> I like display better.

The more I think about it the more I am against any change at all. We 
have some commands with different short- or longopts for help/version. 
These commands hardcoded our old strings and now it looks inconsistent 
again.

So for now I would revert this "include/c.h" part of your patch.

If we still want to change it we should wait until my other usage/stderr 
cleanup settled down. For example I've added now --help and --version 
longopts for *all* our commands. So another idea would be to only show 
the longopts in the short help. Then all tools could use the macros.

Like

   --help     display this help
   --version  display version

... and corresponding short opts will be documented for correctness in 
the man pages only. IMO this would be ok for these trivial options. And 
personally I don't want to be teached about 'fsck -?' or 'chsh -u'. 
Lets hide these random short opts.


> >>  -h, --help     display this help
> >>  -V, --version  show version
> >>
> >> Everybody knows what these options do when they exist.
> >
> > Good points, please, send a patch. It would be also nice to add any
> > comment to the c.h to avoid future updates on this area.
> >
> >     Karel
>
> --
> To unsubscribe from this list: send the line "unsubscribe util-linux"
> in the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/12] hwclock: update --help content and grammar

[J William Piggott]

On 06/21/2017 10:04 PM, Ruediger Meier wrote:
> On Wednesday 21 June 2017, J William Piggott wrote:
>> On 06/21/2017 10:55 AM, Karel Zak wrote:
>>> On Wed, Jun 21, 2017 at 03:05:04PM +0200, Ruediger Meier wrote:
>>>> On Tuesday 20 June 2017, Karel Zak wrote:
>>>>> On Sun, Jun 18, 2017 at 08:45:09PM -0400, J William Piggott wrote:
>>>>>> Signed-off-by: J William Piggott <elseifthen@gmx.com>
>>>>>> ---
>>>>>>  include/c.h         |  4 ++--
>>>>>>  sys-utils/hwclock.c | 48
>>>>>> +++++++++++++++++++++--------------------------- 2 files
>>>>>> changed, 23 insertions(+), 29 deletions(-)
>>>>>>
>>>>>> diff --git a/include/c.h b/include/c.h
>>>>>> index 9c19965..bd073fc 100644
>>>>>> --- a/include/c.h
>>>>>> +++ b/include/c.h
>>>>>> @@ -319,8 +319,8 @@ static inline int xusleep(useconds_t usec)
>>>>>>  #define USAGE_OPTIONS    _("\nOptions:\n")
>>>>>>  #define USAGE_FUNCTIONS  _("\nFunctions:\n")
>>>>>>  #define USAGE_SEPARATOR    "\n"
>>>>>> -#define USAGE_HELP       _(" -h, --help     display this help
>>>>>> and exit\n") -#define USAGE_VERSION    _(" -V, --version  output
>>>>>> version information and exit\n") +#define USAGE_HELP       _("
>>>>>> -h, --help     display this information and exit\n")
>>>>>
>>>>>  "display this help and exit\n"
>>>>> or
>>>>>  "display help information and exit\n"
>>>>>
>>>>> sounds better than "this information".
>>>>
>>>> IMO "this" is important because it makes clear that running --help
>>>> would not give you any better information than the one you see
>>>> already.
>>>>
>>>> Compare this:
>>>> $ xz --help | grep help
>>>>   -h, --help        display this short help and exit
>>>>   -H, --long-help   display the long help (lists also the ...
>>>>
>>>> $ xz --long-help | grep help
>>>>   -h, --help        display the short help (lists only the basic
>>>> ... -H, --long-help   display this long help and exit
>>>>
>>>>
>>>> Our old errtryhelp, USAGE_HELP and USAGE_VERSION were 100% the
>>>> same like coreutils. This was a nice consensus and even a bit
>>>> shorter.
>>>>
>>>> Anyways, instead of improving the grammer and correctness I would
>>>> rather make these trivial option descriptions as short as
>>>> possible, like
>>
>> My intent was to have these two very similar messages using the same
>> language. I think it would be easier to understand for non-native
>> speakers and easier for translators to work with if they did. I'm all
>> for making them more concise, but how about this:
>>
>>   -h, --help     display this help
>>   -V, --version  display version
>> or
>>   -h, --help     show this help
>>   -V, --version  show version
>>
>> I like display better.
> 
> The more I think about it the more I am against any change at all. We 
> have some commands with different short- or longopts for help/version. 
> These commands hardcoded our old strings and now it looks inconsistent 
> again.
> 
> So for now I would revert this "include/c.h" part of your patch.

It's a big job, so if it helps you then it's fine with me.

It would be nice to have help and version use consistent language one day.

> 
> If we still want to change it we should wait until my other usage/stderr 
> cleanup settled down. For example I've added now --help and --version 
> longopts for *all* our commands. So another idea would be to only show 
> the longopts in the short help. Then all tools could use the macros.
> 
> Like
> 
>    --help     display this help
>    --version  display version
> 
> ... and corresponding short opts will be documented for correctness in 
> the man pages only. IMO this would be ok for these trivial options. And 
> personally I don't want to be teached about 'fsck -?' or 'chsh -u'. 
> Lets hide these random short opts.
> 
> 
>>>>  -h, --help     display this help
>>>>  -V, --version  show version
>>>>
>>>> Everybody knows what these options do when they exist.
>>>
>>> Good points, please, send a patch. It would be also nice to add any
>>> comment to the c.h to avoid future updates on this area.
>>>
>>>     Karel
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe util-linux"
>> in the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe util-linux" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 11/12] hwclock: remove unused usage() FILE argument

[J William Piggott]

On 06/21/2017 11:48 AM, J William Piggott wrote:
> 
> 
> On 06/21/2017 05:26 AM, Karel Zak wrote:
>> On Sun, Jun 18, 2017 at 08:49:49PM -0400, J William Piggott wrote:
>>>
>>> Signed-off-by: J William Piggott <elseifthen@gmx.com>
>>> ---
>>>  sys-utils/hwclock.c | 74 ++++++++++++++++++++++++++---------------------------
>>>  1 file changed, 37 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
>>> index 7d69b7a..636d36a 100644
>>> --- a/sys-utils/hwclock.c
>>> +++ b/sys-utils/hwclock.c
>>> @@ -1200,49 +1200,49 @@ static void out_version(void)
>>>  }
>>>  
>>>  static void __attribute__((__noreturn__))
>>> -usage(const struct hwclock_control *ctl, FILE *out)
>>> +usage(const struct hwclock_control *ctl)
>>>  {
>>> -	fputs(USAGE_HEADER, out);
>>> -	fputs(_(" hwclock [function] [option...]\n"), out);
>>> -
>>> -	fputs(USAGE_SEPARATOR, out);
>>> -	fputs(_(" Query or set the RTC (Real Time Clock / Hardware Clock)\n"), out);
>>> -
>>> -	fputs(USAGE_FUNCTIONS, out);
>>> -	fputs(_(" -r, --show           display the RTC time\n"), out);
>>> -	fputs(_("     --get            display drift corrected RTC time\n"), out);
>>> -	fputs(_("     --set            set the RTC according to --date\n"), out);
>>> -	fputs(_(" -s, --hctosys        set the system time from the RTC\n"), out);
>>> -	fputs(_(" -w, --systohc        set the RTC from the system time\n"), out);
>>> -	fputs(_("     --systz          send timescale configurations to the kernel\n"), out);
>>> -	fputs(_("     --adjust         adjust the RTC to account for systematic drift\n"), out);
>>> +	fputs(USAGE_HEADER, stdout);
>>> +	puts(_(" hwclock [function] [option...]"));
>>> +
>>> +	fputs(USAGE_SEPARATOR, stdout);
>>> +	puts(_(" Query or set the RTC (Real Time Clock / Hardware Clock)"));
>>             ^
>> We don't use this space here. Fixed.
> 
> Ah, I miss read this in howto-usage-function.txt:
> 
>> The usage output begins with an empty line, followed by 'Usage:', and
>> then the synopsis on the line after that.  The synopsis and option-
>> description lines are all indented by one space (0x40).
> 
> I read 'synopsis and description', not 'option-description'. Also, it's
> the entire option line, not the option-description that is indented. 
> 
> There is not an example in boilerplate.c for a command description,
> there probably should be.

My memory failed me on this one. There is an example in both docs; it
just wasn't mentioned in the text of howto-usage-function.txt. I will
submit a patch for that and to clarify the indentations.

> 
>>
>>     Karel
>>
> --
> To unsubscribe from this list: send the line "unsubscribe util-linux" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: fputs() vs puts() (was: [PATCH] hwclock: remove unused usage() FILE argument)

[Ruediger Meier]

On Tuesday 20 June 2017, Karel Zak wrote:
> On Sun, Jun 18, 2017 at 08:49:49PM -0400, J William Piggott wrote:
> >  sys-utils/hwclock.c | 74
> > ++++++++++++++++++++++++++--------------------------- 1 file
> > changed, 37 insertions(+), 37 deletions(-)
>
> It would be better to remove this patch from the pull-request; let's
> keep fputs() in the code and wait for any solution from Rudi :-)

I have now finished my cleanup regarding stdout only. To make the diffs 
small I have left a useless definition "FILE *out = stdout;" in almost 
any usage function.

We can remove this "out" variable now everwhere using a sed or awk. But 
a few questions about what would be the best end state.

1. fputs vs puts?:

   Is it a problem for translators if we remove a newline from almost
   any string? And, does puts() look more nice at all? I mean many usage
   functions have to use printf too, so does it look good if some
   strings are '\n' terminated and others not?

2. Alignment for better readabilty in the code.

   I would prefer leading spaces before the first string argument to
   match the longest possible prefix 'printf(_("'. Is that ok?:

   puts(  _(" -q             quiet mode"));
   fputs( _(" -v, --verbose  verbose mode"), stdout);
   printf(_(" -f, --rtc      use alternate to %1$s\n"), _BLA);
   printf(  "     --help     %s\n", USAGE_OPTSTR_HELP);

3. Maybe we should always decouple options and descriptions?

   Introducing a macro like:

   #define prnt_opt(opt, marg_dsc, dsc) \
       printf( "%-" #marg_dsc "s%s\n", opt, dscr)

   /* the magic margin number for the whole function */
   int m = 16;
   prnt_opt(" -q",              m, _("quiet mode");
   prnt_opt(" -v, --verbose",   m, _("verbose mode");
   prnt_opt("      --help",     m, USAGE_OPTSTR_HELP);
   /* or standard help, as exist already */
   print_usage_help_options(m);

4. About our USAGE_* macros

   We can remove the trailing newlines in all macros if puts() is
   preferred (regarding 1.). Or we can just let them print like
   print_usage_help_options().

5. We can leave everything as is and touch all these usage lines only
   if needed. For example I did it without extra noise in a861538c.
   Karel missed his chance in 4fb515f9 ;)

   Note since introducing print_usage_help_options() we *are* already
   mixing hardcoded stdout and variable out. So we can also convert
   single lines from fprintf to printf, like i did in my last pending
   hwclock patch "don't ifdef printf arguments".

cu,
Rudi

Re: fputs() vs puts() (was: [PATCH] hwclock: remove unused usage() FILE argument)

[Karel Zak]

On Wed, Jun 28, 2017 at 09:29:40PM +0200, Ruediger Meier wrote:
> On Tuesday 20 June 2017, Karel Zak wrote:
> > On Sun, Jun 18, 2017 at 08:49:49PM -0400, J William Piggott wrote:
> > >  sys-utils/hwclock.c | 74
> > > ++++++++++++++++++++++++++--------------------------- 1 file
> > > changed, 37 insertions(+), 37 deletions(-)
> >
> > It would be better to remove this patch from the pull-request; let's
> > keep fputs() in the code and wait for any solution from Rudi :-)
> 
> I have now finished my cleanup regarding stdout only. To make the diffs 
> small I have left a useless definition "FILE *out = stdout;" in almost 
> any usage function.
> 
> We can remove this "out" variable now everwhere using a sed or awk. But 
> a few questions about what would be the best end state.
> 
> 1. fputs vs puts?:
> 
>    Is it a problem for translators if we remove a newline from almost
>    any string? And, does puts() look more nice at all? I mean many usage
>    functions have to use printf too, so does it look good if some
>    strings are '\n' terminated and others not?

Frankly, I prefer to have \n in the string, because in this case you
have full control on the output. And it also means all the strings
modification... for me fputs() is the winner :-)

> 2. Alignment for better readabilty in the code.
> 
>    I would prefer leading spaces before the first string argument to
>    match the longest possible prefix 'printf(_("'. Is that ok?:
> 
>    puts(  _(" -q             quiet mode"));
>    fputs( _(" -v, --verbose  verbose mode"), stdout);
>    printf(_(" -f, --rtc      use alternate to %1$s\n"), _BLA);
>    printf(  "     --help     %s\n", USAGE_OPTSTR_HELP);

Yes, good idea.

> 3. Maybe we should always decouple options and descriptions?
> 
>    Introducing a macro like:
> 
>    #define prnt_opt(opt, marg_dsc, dsc) \
>        printf( "%-" #marg_dsc "s%s\n", opt, dscr)
> 
>    /* the magic margin number for the whole function */
>    int m = 16;
>    prnt_opt(" -q",              m, _("quiet mode");
>    prnt_opt(" -v, --verbose",   m, _("verbose mode");
>    prnt_opt("      --help",     m, USAGE_OPTSTR_HELP);
>    /* or standard help, as exist already */
>    print_usage_help_options(m);

Hmm... now translator can also translate option arguments

  --something <time>     this is nice option

in this case <time> is possible to translate too, and maybe in some
languages it's possible that you will need to change number of blank
spaces between option and option description.

IMHO it's against KISS principle :-)

> 4. About our USAGE_* macros
> 
>    We can remove the trailing newlines in all macros if puts() is
>    preferred (regarding 1.). Or we can just let them print like
>    print_usage_help_options().

Frankly, for me it's more readable to have

    fputs(USAGE_OPTIONS, stdout);

than introduce another function. (It was also reason why I was not
sure with print_usage_help_options(), but it resolves more issues.)

> 5. We can leave everything as is and touch all these usage lines only
>    if needed. For example I did it without extra noise in a861538c.
>    Karel missed his chance in 4fb515f9 ;)

It was my goal not to do any change if you work in this area. And yes,
"touch only if needed" is good idea. The blockdev usage() is nice
example that mix more things together is no problem, it's still
readable and without extra complexity (well, maybe use COMMANDS: there :-).

>    Note since introducing print_usage_help_options() we *are* already
>    mixing hardcoded stdout and variable out. So we can also convert
>    single lines from fprintf to printf, like i did in my last pending
>    hwclock patch "don't ifdef printf arguments".

All the issue is libc stupidity, imagine world where puts() is without
\n junk (like printf and fprintf). I think printf() is no problem
there.

For me it's about:

    * nice usage() output
    * usage() function readability
    * all without extra complexity (random contributor has to be able
      add new option easily)
    * translators-friendly (minimize changes, keep it open as much as
      possible)

    Karel

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

Re: fputs() vs puts() (was: [PATCH] hwclock: remove unused usage() FILE argument)

[Ruediger Meier]

On Thursday 29 June 2017, Karel Zak wrote:
> On Wed, Jun 28, 2017 at 09:29:40PM +0200, Ruediger Meier wrote:
> > On Tuesday 20 June 2017, Karel Zak wrote:
> > > On Sun, Jun 18, 2017 at 08:49:49PM -0400, J William Piggott wrote:
> > > >  sys-utils/hwclock.c | 74
> > > > ++++++++++++++++++++++++++--------------------------- 1 file
> > > > changed, 37 insertions(+), 37 deletions(-)
> > >
> > > It would be better to remove this patch from the pull-request;
> > > let's keep fputs() in the code and wait for any solution from
> > > Rudi :-)
> >
> > I have now finished my cleanup regarding stdout only. To make the
> > diffs small I have left a useless definition "FILE *out = stdout;"
> > in almost any usage function.
> >
> > We can remove this "out" variable now everwhere using a sed or awk.
> > But a few questions about what would be the best end state.
> >
> > 1. fputs vs puts?:
> >
> >    Is it a problem for translators if we remove a newline from
> > almost any string? And, does puts() look more nice at all? I mean
> > many usage functions have to use printf too, so does it look good
> > if some strings are '\n' terminated and others not?
>
> Frankly, I prefer to have \n in the string, because in this case you
> have full control on the output. And it also means all the strings
> modification... for me fputs() is the winner :-)
>
> > 2. Alignment for better readabilty in the code.
> >
> >    I would prefer leading spaces before the first string argument
> > to match the longest possible prefix 'printf(_("'. Is that ok?:
> >
> >    puts(  _(" -q             quiet mode"));
> >    fputs( _(" -v, --verbose  verbose mode"), stdout);
> >    printf(_(" -f, --rtc      use alternate to %1$s\n"), _BLA);
> >    printf(  "     --help     %s\n", USAGE_OPTSTR_HELP);
>
> Yes, good idea.
>
> > 3. Maybe we should always decouple options and descriptions?
> >
> >    Introducing a macro like:
> >
> >    #define prnt_opt(opt, marg_dsc, dsc) \
> >        printf( "%-" #marg_dsc "s%s\n", opt, dscr)
> >
> >    /* the magic margin number for the whole function */
> >    int m = 16;
> >    prnt_opt(" -q",              m, _("quiet mode");
> >    prnt_opt(" -v, --verbose",   m, _("verbose mode");
> >    prnt_opt("      --help",     m, USAGE_OPTSTR_HELP);
> >    /* or standard help, as exist already */
> >    print_usage_help_options(m);
>
> Hmm... now translator can also translate option arguments
>
>   --something <time>     this is nice option
>
> in this case <time> is possible to translate too, and maybe in some
> languages it's possible that you will need to change number of blank
> spaces between option and option description.
>
> IMHO it's against KISS principle :-)
>
> > 4. About our USAGE_* macros
> >
> >    We can remove the trailing newlines in all macros if puts() is
> >    preferred (regarding 1.). Or we can just let them print like
> >    print_usage_help_options().
>
> Frankly, for me it's more readable to have
>
>     fputs(USAGE_OPTIONS, stdout);
>
> than introduce another function. (It was also reason why I was not
> sure with print_usage_help_options(), but it resolves more issues.)

I can still change it to be used like MAN_TAIL, then we are consistent 
again.

  print_usage_help_options(16);
  vs.
  printf( USAGE_HELP_OPTIONS(16) );

> > 5. We can leave everything as is and touch all these usage lines
> > only if needed. For example I did it without extra noise in
> > a861538c. Karel missed his chance in 4fb515f9 ;)
>
> It was my goal not to do any change if you work in this area. And
> yes, "touch only if needed" is good idea. The blockdev usage() is
> nice example that mix more things together is no problem, it's still
> readable and without extra complexity (well, maybe use COMMANDS:
> there :-).
>
> >    Note since introducing print_usage_help_options() we *are*
> > already mixing hardcoded stdout and variable out. So we can also
> > convert single lines from fprintf to printf, like i did in my last
> > pending hwclock patch "don't ifdef printf arguments".
>
> All the issue is libc stupidity, imagine world where puts() is
> without \n junk (like printf and fprintf). I think printf() is no
> problem there.
>
> For me it's about:
>
>     * nice usage() output
>     * usage() function readability
>     * all without extra complexity (random contributor has to be able
>       add new option easily)
>     * translators-friendly (minimize changes, keep it open as much as
>       possible)


I see it all like you, so let's stop code cosmetics unless we have real 
changes. I'll send a patch for boilerpalate.c only. and maybe the 
mentioned help_options macro.

cu,
Rudi

Re: fputs() vs puts() (was: [PATCH] hwclock: remove unused usage() FILE argument)

[Karel Zak]

On Thu, Jun 29, 2017 at 10:51:14AM +0200, Karel Zak wrote:
> On Wed, Jun 28, 2017 at 09:29:40PM +0200, Ruediger Meier wrote:
> > On Tuesday 20 June 2017, Karel Zak wrote:
> > > On Sun, Jun 18, 2017 at 08:49:49PM -0400, J William Piggott wrote:
> > > >  sys-utils/hwclock.c | 74
> > > > ++++++++++++++++++++++++++--------------------------- 1 file
> > > > changed, 37 insertions(+), 37 deletions(-)
> > >
> > > It would be better to remove this patch from the pull-request; let's
> > > keep fputs() in the code and wait for any solution from Rudi :-)
> > 
> > I have now finished my cleanup regarding stdout only. To make the diffs 
> > small I have left a useless definition "FILE *out = stdout;" in almost 
> > any usage function.
> > 
> > We can remove this "out" variable now everwhere using a sed or awk. But 
> > a few questions about what would be the best end state.
> > 
> > 1. fputs vs puts?:
> > 
> >    Is it a problem for translators if we remove a newline from almost
> >    any string? And, does puts() look more nice at all? I mean many usage
> >    functions have to use printf too, so does it look good if some
> >    strings are '\n' terminated and others not?
> 
> Frankly, I prefer to have \n in the string, because in this case you
> have full control on the output. And it also means all the strings
> modification... for me fputs() is the winner :-)

Note that I don't think we have to be so strict about it. For old already 
translated strings I prefer fputs to avoid \n changes, but for new
tools or after some massive change in usage() it's probbaly fine to
use puts(). Use common sense :-)

    Karel


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

Re: fputs() vs puts() (was: [PATCH] hwclock: remove unused usage() FILE argument)

[Karel Zak]

On Thu, Jun 29, 2017 at 12:37:19PM +0200, Ruediger Meier wrote:
> > than introduce another function. (It was also reason why I was not
> > sure with print_usage_help_options(), but it resolves more issues.)
> 
> I can still change it to be used like MAN_TAIL, then we are consistent 
> again.
> 
>   print_usage_help_options(16);
>   vs.
>   printf( USAGE_HELP_OPTIONS(16) );

Yes, I have no strong opinion about it, but somehow to have libc stuff
without any extra abstraction in the code seems always better.

> > For me it's about:
> >
> >     * nice usage() output
> >     * usage() function readability
> >     * all without extra complexity (random contributor has to be able
> >       add new option easily)
> >     * translators-friendly (minimize changes, keep it open as much as
> >       possible)
> 
> 
> I see it all like you, so let's stop code cosmetics unless we have real 
> changes. I'll send a patch for boilerpalate.c only. and maybe the 
> mentioned help_options macro.

I just pushed wipefs and uuidgen with puts(), it seems good. So my
suggestion

 1) use libc output functions if possible
 2) use puts() for new tools or after massive changes in usage()
 3) don't use puts() for already translated usage()
 4) use  
        --output <list>    COLUMNS to display (see below)
        ...
        COLUMNS:


   (William, can you update your "standardize USAGE_COLUMNS" patches? Please.)

 5) "perfect is the enemy of good" (your new (or first) tatto...)

 6) keep our maintainer happy and send patches with new regression
    tests, improve stability, implement new features, etc. :)

    Karel

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

Re: fputs() vs puts()

[J William Piggott]

On 06/29/2017 06:53 AM, Karel Zak wrote:
> On Thu, Jun 29, 2017 at 12:37:19PM +0200, Ruediger Meier wrote:
>>> than introduce another function. (It was also reason why I was not
>>> sure with print_usage_help_options(), but it resolves more issues.)
>>
>> I can still change it to be used like MAN_TAIL, then we are consistent 
>> again.
>>
>>   print_usage_help_options(16);
>>   vs.
>>   printf( USAGE_HELP_OPTIONS(16) );
> 
> Yes, I have no strong opinion about it, but somehow to have libc stuff
> without any extra abstraction in the code seems always better.
> 
>>> For me it's about:
>>>
>>>     * nice usage() output
>>>     * usage() function readability
>>>     * all without extra complexity (random contributor has to be able
>>>       add new option easily)
>>>     * translators-friendly (minimize changes, keep it open as much as
>>>       possible)
>>
>>
>> I see it all like you, so let's stop code cosmetics unless we have real 
>> changes. I'll send a patch for boilerpalate.c only. and maybe the 
>> mentioned help_options macro.
> 
> I just pushed wipefs and uuidgen with puts(), it seems good. So my
> suggestion
> 
>  1) use libc output functions if possible
>  2) use puts() for new tools or after massive changes in usage()
>  3) don't use puts() for already translated usage()
>  4) use  
>         --output <list>    COLUMNS to display (see below)
>         ...
>         COLUMNS:
> 
> 
>    (William, can you update your "standardize USAGE_COLUMNS" patches? Please.)

Sure, one clarification please: you want only the columns header in all
caps? So:

Usage:
Options:
Functions:
Commands:
COLUMNS:


> 
>  5) "perfect is the enemy of good" (your new (or first) tatto...)
> 
>  6) keep our maintainer happy and send patches with new regression
>     tests, improve stability, implement new features, etc. :)
> 
>     Karel
> 

Re: fputs() vs puts()

[Karel Zak]

On Thu, Jun 29, 2017 at 10:46:50AM -0400, J William Piggott wrote:
> >    (William, can you update your "standardize USAGE_COLUMNS" patches? Please.)
> 
> Sure, one clarification please: you want only the columns header in all
> caps? So:
> 
> Usage:
> Options:
> Functions:
> Commands:
> COLUMNS:

ha.. that's a good question. Suggestions? 

For example coreutils project uses uppercase for keywords only, for
example dd(1)

    Each FLAG symbol may be:
       ...
    Each CONV symbol may be:
       ...

it seems like original Rudi's idea

    Available COLUMNS:

it's more compatible with the rest than alone "COLUMNS". So I vote
this one.

    Karel

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

Re: fputs() vs puts()

[J William Piggott]

On 06/29/2017 04:12 PM, Karel Zak wrote:
> On Thu, Jun 29, 2017 at 10:46:50AM -0400, J William Piggott wrote:
>>>    (William, can you update your "standardize USAGE_COLUMNS" patches? Please.)
>>
>> Sure, one clarification please: you want only the columns header in all
>> caps? So:
>>
>> Usage:
>> Options:
>> Functions:
>> Commands:
>> COLUMNS:
> 
> ha.. that's a good question. Suggestions? 
> 
> For example coreutils project uses uppercase for keywords only, for
> example dd(1)
> 
>     Each FLAG symbol may be:
>        ...
>     Each CONV symbol may be:
>        ...
> 
> it seems like original Rudi's idea
> 
>     Available COLUMNS:


But they also use:
of=FILE
conv=CONVS

So we cannot completely follow that paradigm. The above are not option
args, and you do not want option args in all caps.

The columns situation is unique, we don't have to play follow the leader.

Comparing:

        --output <list>    COLUMNS to display (see below)
        ...
        Available COLUMNS:

With:
        --output <list>    columns to display (see Columns:)
        ...
        Columns:


What does the reader learn by yelling COLUMNS at them. Sometimes a vague
reference like 'see below' is required. In this case the reader can be
told exactly what they a looking for. It also keeps our usage() headings
consistent.

The two are not so different; I just think my solution is less
ambiguous, is easier to read, and looks a bit cleaner.

It's your call to make Karel.


> 
> it's more compatible with the rest than alone "COLUMNS". So I vote
> this one.
> 
>     Karel
> 

Re: fputs() vs puts()

[Ruediger Meier]

On Friday 30 June 2017, J William Piggott wrote:
> On 06/29/2017 04:12 PM, Karel Zak wrote:
> > On Thu, Jun 29, 2017 at 10:46:50AM -0400, J William Piggott wrote:
> >>>    (William, can you update your "standardize USAGE_COLUMNS"
> >>> patches? Please.)
> >>
> >> Sure, one clarification please: you want only the columns header
> >> in all caps? So:
> >>
> >> Usage:
> >> Options:
> >> Functions:
> >> Commands:
> >> COLUMNS:
> >
> > ha.. that's a good question. Suggestions?
> >
> > For example coreutils project uses uppercase for keywords only, for
> > example dd(1)
> >
> >     Each FLAG symbol may be:
> >        ...
> >     Each CONV symbol may be:
> >        ...
> >
> > it seems like original Rudi's idea
> >
> >     Available COLUMNS:
>
> But they also use:
> of=FILE
> conv=CONVS
>
> So we cannot completely follow that paradigm. The above are not
> option args, and you do not want option args in all caps.
>
> The columns situation is unique, we don't have to play follow the
> leader.
>
> Comparing:
>
>         --output <list>    COLUMNS to display (see below)
>         ...
>         Available COLUMNS:
>
> With:
>         --output <list>    columns to display (see Columns:)
>         ...
>         Columns:
>
>
> What does the reader learn by yelling COLUMNS at them.

It's just de-facto standard for "--help/man"-like technical 
documentation. If you would write html or info docs, then you would not 
use capital letters but links to click on.

Compare:

$ man bash | grep -i EXPANSION
$ man bash | grep  EXPANSION

or

$ xz --help | grep -i FILE
$ xz --help | grep FILE 

It's a very useful style and users *use* this by case-sensitive search 
in less or grep. This style is not always used 100% consistently but 
I'm happy whenever I see it, saves me a lot of time usually.

cu,
Rudi

> Sometimes a 
> vague reference like 'see below' is required. In this case the reader
> can be told exactly what they a looking for. It also keeps our
> usage() headings consistent.
>
> The two are not so different; I just think my solution is less
> ambiguous, is easier to read, and looks a bit cleaner.
>
> It's your call to make Karel.
>
> > it's more compatible with the rest than alone "COLUMNS". So I vote
> > this one.
> >
> >     Karel

Re: fputs() vs puts()

[Karel Zak]

On Mon, Jul 03, 2017 at 10:30:21AM +0200, Ruediger Meier wrote:
> On Friday 30 June 2017, J William Piggott wrote:
> > On 06/29/2017 04:12 PM, Karel Zak wrote:
> > > On Thu, Jun 29, 2017 at 10:46:50AM -0400, J William Piggott wrote:
> > >>>    (William, can you update your "standardize USAGE_COLUMNS"
> > >>> patches? Please.)
> > >>
> > >> Sure, one clarification please: you want only the columns header
> > >> in all caps? So:
> > >>
> > >> Usage:
> > >> Options:
> > >> Functions:
> > >> Commands:
> > >> COLUMNS:
> > >
> > > ha.. that's a good question. Suggestions?
> > >
> > > For example coreutils project uses uppercase for keywords only, for
> > > example dd(1)
> > >
> > >     Each FLAG symbol may be:
> > >        ...
> > >     Each CONV symbol may be:
> > >        ...
> > >
> > > it seems like original Rudi's idea
> > >
> > >     Available COLUMNS:
> >
> > But they also use:
> > of=FILE
> > conv=CONVS
> >
> > So we cannot completely follow that paradigm. The above are not
> > option args, and you do not want option args in all caps.
> >
> > The columns situation is unique, we don't have to play follow the
> > leader.
> >
> > Comparing:
> >
> >         --output <list>    COLUMNS to display (see below)
> >         ...
> >         Available COLUMNS:

For me it looks better (including esthetical point of view ;-)

But wait with it...

> > With:
> >         --output <list>    columns to display (see Columns:)
> >         ...
> >         Columns:
> >
> >
> > What does the reader learn by yelling COLUMNS at them.
>
> It's just de-facto standard for "--help/man"-like technical 
> documentation. If you would write html or info docs, then you would not 
> use capital letters but links to click on.
> 
> Compare:
> 
> $ man bash | grep -i EXPANSION
> $ man bash | grep  EXPANSION
> 
> or
> 
> $ xz --help | grep -i FILE
> $ xz --help | grep FILE 
> 
> It's a very useful style and users *use* this by case-sensitive search 
> in less or grep. This style is not always used 100% consistently but 
> I'm happy whenever I see it, saves me a lot of time usually.

 Maybe it would be possible to convince me that coreutils way for
 options arguments is the right way :-))
 
 I mean:

   --output COLUMNS      
   --length SIZE
   --net[=FILE]

 ...etc. The question is how to compose the option string to minimize
 duplication and inform about a format, for example

   --output COLUMNS    list of columns to display (see below)

  
 Note that on many places we for example for SIZE accept suffixes
 (MiB, GiB...) but this info is nowhere in the --help output.

 And note that I like the William's idea with the explicit reader
 redirection "(see below)".

 Maybe Rudy is right and it's time to think about it for this release
 if we already decided to cleanup usage().

    Karel

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

Re: fputs() vs puts()

[Ruediger Meier]

On Monday 03 July 2017, Karel Zak wrote:
> On Mon, Jul 03, 2017 at 10:30:21AM +0200, Ruediger Meier wrote:
> > On Friday 30 June 2017, J William Piggott wrote:
> > > On 06/29/2017 04:12 PM, Karel Zak wrote:
> > > > On Thu, Jun 29, 2017 at 10:46:50AM -0400, J William Piggott wrote:
> > > >>>    (William, can you update your "standardize USAGE_COLUMNS"
> > > >>> patches? Please.)
> > > >>
> > > >> Sure, one clarification please: you want only the columns
> > > >> header in all caps? So:
> > > >>
> > > >> Usage:
> > > >> Options:
> > > >> Functions:
> > > >> Commands:
> > > >> COLUMNS:
> > > >
> > > > ha.. that's a good question. Suggestions?
> > > >
> > > > For example coreutils project uses uppercase for keywords only,
> > > > for example dd(1)
> > > >
> > > >     Each FLAG symbol may be:
> > > >        ...
> > > >     Each CONV symbol may be:
> > > >        ...
> > > >
> > > > it seems like original Rudi's idea
> > > >
> > > >     Available COLUMNS:
> > >
> > > But they also use:
> > > of=FILE
> > > conv=CONVS
> > >
> > > So we cannot completely follow that paradigm. The above are not
> > > option args, and you do not want option args in all caps.
> > >
> > > The columns situation is unique, we don't have to play follow the
> > > leader.
> > >
> > > Comparing:
> > >
> > >         --output <list>    COLUMNS to display (see below)
> > >         ...
> > >         Available COLUMNS:
>
> For me it looks better (including esthetical point of view ;-)
>
> But wait with it...
>
> > > With:
> > >         --output <list>    columns to display (see Columns:)
> > >         ...
> > >         Columns:
> > >
> > >
> > > What does the reader learn by yelling COLUMNS at them.
> >
> > It's just de-facto standard for "--help/man"-like technical
> > documentation. If you would write html or info docs, then you would
> > not use capital letters but links to click on.
> >
> > Compare:
> >
> > $ man bash | grep -i EXPANSION
> > $ man bash | grep  EXPANSION
> >
> > or
> >
> > $ xz --help | grep -i FILE
> > $ xz --help | grep FILE
> >
> > It's a very useful style and users *use* this by case-sensitive
> > search in less or grep. This style is not always used 100%
> > consistently but I'm happy whenever I see it, saves me a lot of
> > time usually.
>
>  Maybe it would be possible to convince me that coreutils way for
>  options arguments is the right way :-))
>
>  I mean:
>
>    --output COLUMNS
>    --length SIZE
>    --net[=FILE]
>
>  ...etc. The question is how to compose the option string to minimize
>  duplication and inform about a format, for example
>
>    --output COLUMNS    list of columns to display (see below)
>
>
>  Note that on many places we for example for SIZE accept suffixes
>  (MiB, GiB...) but this info is nowhere in the --help output.

Coreutils is using some of these capitalized words really as
system-wide keywords. For example SIZE:

  The SIZE argument is an integer and optional unit (example: 10K is 10*1024).
  Units are K,M,G,T,P,E,Z,Y (powers of 1024) or KB,MB,... (powers of 1000).

BTW this is my only coreutils patch to make the size description
human understandable and let it fit into two 80 char columns. ;)

There was a big discussion about this here in bug#9939
https://lists.gnu.org/archive/html/bug-coreutils/2011-11/threads.html#00002


>  And note that I like the William's idea with the explicit reader
>  redirection "(see below)".

Yep, IMO the "see below" is good *always* if the overall text is a bit
longer. In case there are only a few lines or if obvious (like FILE), it
*may* be skipped, I would say.

>  Maybe Rudy is right and it's time to think about it for this release
>  if we already decided to cleanup usage().
>
>     Karel

Re: fputs() vs puts()

[Karel Zak]

On Mon, Jul 03, 2017 at 02:38:33PM +0200, Ruediger Meier wrote:
> On Monday 03 July 2017, Karel Zak wrote:
> >  Maybe it would be possible to convince me that coreutils way for
> >  options arguments is the right way :-))
> >
> >  I mean:
> >
> >    --output COLUMNS
> >    --length SIZE
> >    --net[=FILE]
> >
> >  ...etc. The question is how to compose the option string to minimize
> >  duplication and inform about a format, for example
> >
> >    --output COLUMNS    list of columns to display (see below)
> >
> >
> >  Note that on many places we for example for SIZE accept suffixes
> >  (MiB, GiB...) but this info is nowhere in the --help output.
> 
> Coreutils is using some of these capitalized words really as
> system-wide keywords. For example SIZE:
> 
>   The SIZE argument is an integer and optional unit (example: 10K is 10*1024).
>   Units are K,M,G,T,P,E,Z,Y (powers of 1024) or KB,MB,... (powers of 1000).
> 
> BTW this is my only coreutils patch to make the size description
> human understandable and let it fit into two 80 char columns. ;)
> 
> There was a big discussion about this here in bug#9939
> https://lists.gnu.org/archive/html/bug-coreutils/2011-11/threads.html#00002
> 
> 
> >  And note that I like the William's idea with the explicit reader
> >  redirection "(see below)".
> 
> Yep, IMO the "see below" is good *always* if the overall text is a bit
> longer. In case there are only a few lines or if obvious (like FILE), it
> *may* be skipped, I would say.

I agree. What about the section where we explain the references? I
personally prefer sentence-like way:

   Available COLUMNS:
      ...
   The SIZE argument...

More complex example (lsblk):

 now:
   -e, --exclude <list>    exclude devices by major number (default: RAM disks)
   -I, --include <list>    show only devices with specified major numbers
   -o, --output <list>     output columns
   -x, --sort <column>     sort output by <column>


 after change:
   -e, --exclude MAJOR     exclude specified devices (see below)
   -I, --include MAJOR     show only specified devices (see below)
   -o, --output COLUMNS    output columns (see below)
   -x, --sort COLUMN       sort output by column

 The MAJOR command separated list of the device major numbers. The
 ramdisk devices are excluded by default.

 Available COLUMNS:
   NAME  device name
   KNAME  internal kernel device name
   ...


Objections, comment? 

    Karel

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

Re: fputs() vs puts()

[Bernhard Voelker]

On 07/03/2017 04:25 PM, Karel Zak wrote:
>  after change:
>    -e, --exclude MAJOR     exclude specified devices (see below)

> Objections, comment? 

+1 on the above.

Not to open a can of works, but what about documenting option-arguments
to long options separated with '=' in between?

  PRG --opt LIST ARG
vs.
  PRG --opt=LIST ARG

While getopt_long() supports both, the latters is unambigous with regard
to whether LIST is an argument-option or another argument to the program.
The difference may be important when a program changes an option to
accept option-arguments.

Thanks & have a nice day,
Berny

Re: fputs() vs puts()

[Bernhard Voelker]

On 07/03/2017 05:09 PM, Bernhard Voelker wrote:
> Not to open a can of works,
________________^^^^^^^^^^^^

he,he nice typo ... but it seems to be okay in this context.  ;-)