Util-Linux package development
 help / color / mirror / Atom feed
* [PATCH 00/13] pull: rtcwake changes
@ 2015-02-22 14:42 Sami Kerola
  2015-02-22 14:42 ` [PATCH 01/13] rtcwake: add rtcwake_control and remove global variables Sami Kerola
                   ` (13 more replies)
  0 siblings, 14 replies; 29+ messages in thread
From: Sami Kerola @ 2015-02-22 14:42 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

Hello,

The following changes are clean ups and improvements to rtcwake command.
These changes are also available from my github remote repository.

 git://github.com/kerolasa/lelux-utiliteetit.git rtcwake

Sami Kerola (13):
  rtcwake: add rtcwake_control and remove global variables
  rtcwake: enumerate constant mode strings
  rtcwake: replace long if else statement with switch case
  rtcwake: remove RTC_ALM_READ and RTC_ALM_SET compatibility
  rtcwake: improve read_clock_mode()
  rtcwake: add human readable --date timestamp format
  rtcwake: fix preprocessor redefinition
  rtcwake: clean up struct tm initializations
  rtcwake: do not overwrite device name
  bash-completion: add freeze mode to rtcwake
  rtcwake: improve coding style
  rtcwake: make some command line options mutually exclusive
  rtcwake: read accepted mode strings from /sys/power/state

 bash-completion/rtcwake |   9 +-
 sys-utils/rtcwake.8.in  |  19 +-
 sys-utils/rtcwake.c     | 579 ++++++++++++++++++++++--------------------------
 3 files changed, 290 insertions(+), 317 deletions(-)

-- 
2.3.0


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

* [PATCH 01/13] rtcwake: add rtcwake_control and remove global variables
  2015-02-22 14:42 [PATCH 00/13] pull: rtcwake changes Sami Kerola
@ 2015-02-22 14:42 ` Sami Kerola
  2015-02-22 14:42 ` [PATCH 02/13] rtcwake: enumerate constant mode strings Sami Kerola
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2015-02-22 14:42 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

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

diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
index bf99a5b..8b5f69c 100644
--- a/sys-utils/rtcwake.c
+++ b/sys-utils/rtcwake.c
@@ -54,16 +54,21 @@
 #define DEFAULT_DEVICE		"/dev/rtc0"
 #define DEFAULT_MODE		"standby"
 
-enum ClockMode {
+enum clock_modes {
 	CM_AUTO,
 	CM_UTC,
 	CM_LOCAL
 };
 
-static char		*adjfile = _PATH_ADJTIME;
-static unsigned		verbose;
-static unsigned		dryrun;
-enum ClockMode		clock_mode = CM_AUTO;
+struct rtcwake_control {
+	char *adjfile;			/* adjtime file path */
+	enum clock_modes clock_mode;	/* hwclock timezone */
+	time_t sys_time;		/* system time */
+	time_t rtc_time;		/* hardware time */
+	unsigned int
+	 verbose:1,			/* verbose messaging */
+	 dryrun:1;			/* do not set alarm, suspend system, etc */
+};
 
 static void __attribute__((__noreturn__)) usage(FILE *out)
 {
@@ -123,11 +128,7 @@ static int is_wakeup_enabled(const char *devname)
 	return strcmp(buf, "enabled") == 0;
 }
 
-/* all times should be in UTC */
-static time_t	sys_time;
-static time_t	rtc_time;
-
-static int get_basetimes(int fd)
+static int get_basetimes(struct rtcwake_control *ctl, int fd)
 {
 	struct tm	tm;
 	struct rtc_time	rtc;
@@ -135,7 +136,7 @@ static int get_basetimes(int fd)
 	/* this process works in RTC time, except when working
 	 * with the system clock (which always uses UTC).
 	 */
-	if (clock_mode == CM_UTC)
+	if (ctl->clock_mode == CM_UTC)
 		setenv("TZ", "UTC", 1);
 	tzset();
 
@@ -146,8 +147,8 @@ static int get_basetimes(int fd)
 		warn(_("read rtc time failed"));
 		return -1;
 	}
-	sys_time = time(0);
-	if (sys_time == (time_t)-1) {
+	ctl->sys_time = time(0);
+	if (ctl->sys_time == (time_t)-1) {
 		warn(_("read system time failed"));
 		return -1;
 	}
@@ -163,33 +164,33 @@ static int get_basetimes(int fd)
 	tm.tm_mon = rtc.tm_mon;
 	tm.tm_year = rtc.tm_year;
 	tm.tm_isdst = -1;  /* assume the system knows better than the RTC */
-	rtc_time = mktime(&tm);
+	ctl->rtc_time = mktime(&tm);
 
-	if (rtc_time == (time_t)-1) {
+	if (ctl->rtc_time == (time_t)-1) {
 		warn(_("convert rtc time failed"));
 		return -1;
 	}
 
-	if (verbose) {
+	if (ctl->verbose) {
 		/* Unless the system uses UTC, either delta or tzone
 		 * reflects a seconds offset from UTC.  The value can
 		 * help sort out problems like bugs in your C library.
 		 */
-		printf("\tdelta   = %ld\n", sys_time - rtc_time);
+		printf("\tdelta   = %ld\n", ctl->sys_time - ctl->rtc_time);
 		printf("\ttzone   = %ld\n", timezone);
 
 		printf("\ttzname  = %s\n", tzname[daylight]);
-		gmtime_r(&rtc_time, &tm);
+		gmtime_r(&ctl->rtc_time, &tm);
 		printf("\tsystime = %ld, (UTC) %s",
-				(long) sys_time, asctime(gmtime(&sys_time)));
+				(long) ctl->sys_time, asctime(gmtime(&ctl->sys_time)));
 		printf("\trtctime = %ld, (UTC) %s",
-				(long) rtc_time, asctime(&tm));
+				(long) ctl->rtc_time, asctime(&tm));
 	}
 
 	return 0;
 }
 
-static int setup_alarm(int fd, time_t *wakeup)
+static int setup_alarm(struct rtcwake_control *ctl, int fd, time_t *wakeup)
 {
 	struct tm		*tm;
 	struct rtc_wkalrm	wake;
@@ -220,11 +221,11 @@ static int setup_alarm(int fd, time_t *wakeup)
 	wake.enabled = 1;
 
 	/* First try the preferred RTC_WKALM_SET */
-	if (!dryrun && ioctl(fd, RTC_WKALM_SET, &wake) < 0) {
+	if (!ctl->dryrun && ioctl(fd, RTC_WKALM_SET, &wake) < 0) {
 		wake.enabled = 0;
 		/* Fall back on the non-preferred way of setting wakeups; only
 		* works for alarms < 24 hours from now */
-		if ((rtc_time + (24 * 60 * 60)) > *wakeup) {
+		if ((ctl->rtc_time + (24 * 60 * 60)) > *wakeup) {
 			if (ioctl(fd, RTC_ALM_SET, &wake.time) < 0) {
 				warn(_("set rtc alarm failed"));
 				return -1;
@@ -260,7 +261,7 @@ static int is_suspend_available(const char *suspend)
 	return rc;
 }
 
-static void suspend_system(const char *suspend)
+static void suspend_system(struct rtcwake_control *ctl, const char *suspend)
 {
 	FILE	*f = fopen(SYS_POWER_STATE_PATH, "w");
 
@@ -269,7 +270,7 @@ static void suspend_system(const char *suspend)
 		return;
 	}
 
-	if (!dryrun) {
+	if (!ctl->dryrun) {
 		fprintf(f, "%s\n", suspend);
 		fflush(f);
 	}
@@ -280,12 +281,12 @@ static void suspend_system(const char *suspend)
 }
 
 
-static int read_clock_mode(void)
+static int read_clock_mode(struct rtcwake_control *ctl)
 {
 	FILE *fp;
 	char linebuf[MAX_LINE];
 
-	fp = fopen(adjfile, "r");
+	fp = fopen(ctl->adjfile, "r");
 	if (!fp)
 		return -1;
 
@@ -308,9 +309,9 @@ static int read_clock_mode(void)
 	}
 
 	if (strncmp(linebuf, "UTC", 3) == 0)
-		clock_mode = CM_UTC;
+		ctl->clock_mode = CM_UTC;
 	else if (strncmp(linebuf, "LOCAL", 5) == 0)
-		clock_mode = CM_LOCAL;
+		ctl->clock_mode = CM_LOCAL;
 
 	fclose(fp);
 
@@ -320,7 +321,7 @@ static int read_clock_mode(void)
 /**
  * print basic alarm settings
  */
-static int print_alarm(int fd)
+static int print_alarm(struct rtcwake_control *ctl, int fd)
 {
 	struct rtc_wkalrm wake;
 	struct rtc_time rtc;
@@ -365,7 +366,7 @@ static int print_alarm(int fd)
 	}
 
 	/* 0 if both UTC, or expresses diff if RTC in local time */
-	alarm += sys_time - rtc_time;
+	alarm += ctl->sys_time - ctl->rtc_time;
 
 	printf(_("alarm: on  %s"), ctime(&alarm));
 	return 0;
@@ -373,6 +374,12 @@ static int print_alarm(int fd)
 
 int main(int argc, char **argv)
 {
+	struct rtcwake_control ctl = {
+		.adjfile = _PATH_ADJTIME,
+		.clock_mode = CM_AUTO,
+		0
+	};
+
 	char		*devname = DEFAULT_DEVICE;
 	unsigned	seconds = 0;
 	char		*suspend = DEFAULT_MODE;
@@ -408,7 +415,7 @@ int main(int argc, char **argv)
 		switch (t) {
 		case 'A':
 			/* for better compatibility with hwclock */
-			adjfile = optarg;
+			ctl.adjfile = optarg;
 			break;
 		case 'a':
 			/* CM_AUTO is default */
@@ -419,7 +426,7 @@ int main(int argc, char **argv)
 			break;
 
 		case 'l':
-			clock_mode = CM_LOCAL;
+			ctl.clock_mode = CM_LOCAL;
 			break;
 
 			/* what system power mode to use?  for now handle only
@@ -450,7 +457,7 @@ int main(int argc, char **argv)
 			break;
 
 		case 'n':
-			dryrun = 1;
+			ctl.dryrun = 1;
 			break;
 
 			/* alarm time, seconds-to-sleep (relative) */
@@ -466,11 +473,11 @@ int main(int argc, char **argv)
 			break;
 
 		case 'u':
-			clock_mode = CM_UTC;
+			ctl.clock_mode = CM_UTC;
 			break;
 
 		case 'v':
-			verbose++;
+			ctl.verbose = 1;
 			break;
 
 		case 'V':
@@ -484,15 +491,15 @@ int main(int argc, char **argv)
 		}
 	}
 
-	if (clock_mode == CM_AUTO) {
-		if (read_clock_mode() < 0) {
+	if (ctl.clock_mode == CM_AUTO) {
+		if (read_clock_mode(&ctl) < 0) {
 			printf(_("%s: assuming RTC uses UTC ...\n"),
 					program_invocation_short_name);
-			clock_mode = CM_UTC;
+			ctl.clock_mode = CM_UTC;
 		}
 	}
-	if (verbose)
-		printf("%s", clock_mode == CM_UTC ? _("Using UTC time.\n") :
+	if (ctl.verbose)
+		printf("%s",  ctl.clock_mode == CM_UTC ? _("Using UTC time.\n") :
 				_("Using local time.\n"));
 
 	if (!alarm && !seconds && strcmp(suspend,"disable") &&
@@ -527,11 +534,11 @@ int main(int argc, char **argv)
 		err(EXIT_FAILURE, _("cannot open %s"), devname);
 
 	/* relative or absolute alarm time, normalized to time_t */
-	if (get_basetimes(fd) < 0)
+	if (get_basetimes(&ctl, fd) < 0)
 		exit(EXIT_FAILURE);
-	if (verbose)
+	if (ctl.verbose)
 		printf(_("alarm %ld, sys_time %ld, rtc_time %ld, seconds %u\n"),
-				alarm, sys_time, rtc_time, seconds);
+				alarm, ctl.sys_time, ctl.rtc_time, seconds);
 
 	if (strcmp(suspend, "show") && strcmp(suspend, "disable")) {
 		if (strcmp(suspend, "no") && strcmp(suspend, "on") &&
@@ -543,14 +550,14 @@ int main(int argc, char **argv)
 		 * modes are not set
 		 */
 		if (alarm) {
-			if (alarm < sys_time)
+			if (alarm < ctl.sys_time)
 				errx(EXIT_FAILURE, _("time doesn't go backward to %s"),
 						ctime(&alarm));
-			alarm += sys_time - rtc_time;
+			alarm += ctl.sys_time - ctl.rtc_time;
 		} else
-			alarm = rtc_time + seconds + 1;
+			alarm = ctl.rtc_time + seconds + 1;
 
-		if (setup_alarm(fd, &alarm) < 0)
+		if (setup_alarm(&ctl, fd, &alarm) < 0)
 			exit(EXIT_FAILURE);
 
 		if (strcmp(suspend, "no") == 0 || strcmp(suspend, "on") == 0)
@@ -566,15 +573,15 @@ int main(int argc, char **argv)
 	}
 
 	if (strcmp(suspend, "no") == 0) {
-		if (verbose)
+		if (ctl.verbose)
 			printf(_("suspend mode: no; leaving\n"));
-		dryrun = 1;	/* to skip disabling alarm at the end */
+		ctl.dryrun = 1;	/* to skip disabling alarm at the end */
 
 	} else if (strcmp(suspend, "off") == 0) {
 		char *arg[5];
 		int i = 0;
 
-		if (verbose)
+		if (ctl.verbose)
 			printf(_("suspend mode: off; executing %s\n"),
 						_PATH_SHUTDOWN);
 		arg[i++] = _PATH_SHUTDOWN;
@@ -583,7 +590,7 @@ int main(int argc, char **argv)
 		arg[i++] = "now";
 		arg[i]   = NULL;
 
-		if (!dryrun) {
+		if (!ctl.dryrun) {
 			execv(arg[0], arg);
 
 			warn(_("failed to execute %s"), _PATH_SHUTDOWN);
@@ -593,41 +600,41 @@ int main(int argc, char **argv)
 	} else if (strcmp(suspend, "on") == 0) {
 		unsigned long data;
 
-		if (verbose)
+		if (ctl.verbose)
 			printf(_("suspend mode: on; reading rtc\n"));
 
-		if (!dryrun) {
+		if (!ctl.dryrun) {
 			do {
 				t = read(fd, &data, sizeof data);
 				if (t < 0) {
 					warn(_("rtc read failed"));
 					break;
 				}
-				if (verbose)
+				if (ctl.verbose)
 					printf("... %s: %03lx\n", devname, data);
 			} while (!(data & RTC_AF));
 		}
 
 	} else if (strcmp(suspend, "disable") == 0) {
 		/* just break, alarm gets disabled in the end */
-		if (verbose)
+		if (ctl.verbose)
 			printf(_("suspend mode: disable; disabling alarm\n"));
 
 	} else if(strcmp(suspend,"show") == 0) {
-		if (verbose)
+		if (ctl.verbose)
 			printf(_("suspend mode: show; printing alarm info\n"));
-		if (print_alarm(fd))
+		if (print_alarm(&ctl, fd))
 			rc = EXIT_FAILURE;
-		dryrun = 1;	/* don't really disable alarm in the end, just show */
+		ctl.dryrun = 1;	/* don't really disable alarm in the end, just show */
 
 	} else {
-		if (verbose)
+		if (ctl.verbose)
 			printf(_("suspend mode: %s; suspending system\n"), suspend);
 		sync();
-		suspend_system(suspend);
+		suspend_system(&ctl, suspend);
 	}
 
-	if (!dryrun) {
+	if (!ctl.dryrun) {
 		/* try to disable the alarm with the preferred RTC_WKALM_RD and
 		 * RTC_WKALM_SET calls, if it fails fall back to RTC_AIE_OFF
 		 */
-- 
2.3.0


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

* [PATCH 02/13] rtcwake: enumerate constant mode strings
  2015-02-22 14:42 [PATCH 00/13] pull: rtcwake changes Sami Kerola
  2015-02-22 14:42 ` [PATCH 01/13] rtcwake: add rtcwake_control and remove global variables Sami Kerola
@ 2015-02-22 14:42 ` Sami Kerola
  2015-02-24 12:34   ` Karel Zak
  2015-02-22 14:42 ` [PATCH 03/13] rtcwake: replace long if else statement with switch case Sami Kerola
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Sami Kerola @ 2015-02-22 14:42 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

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

diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
index 8b5f69c..372e620 100644
--- a/sys-utils/rtcwake.c
+++ b/sys-utils/rtcwake.c
@@ -52,7 +52,37 @@
 #define RTC_PATH		"/sys/class/rtc/%s/device/power/wakeup"
 #define SYS_POWER_STATE_PATH	"/sys/power/state"
 #define DEFAULT_DEVICE		"/dev/rtc0"
-#define DEFAULT_MODE		"standby"
+
+enum rtc_modes {	/* manual page --mode option explains these. */
+	STANDBY_MODE = 0,
+	MEM_MODE,
+	FREEZE_MODE,
+	DISK_MODE,	/* end of Documentation/power/states.txt modes  */
+	OFF_MODE,
+	NO_MODE,
+	ON_MODE,	/* smaller <- read the code */
+	DISABLE_MODE,	/* greater <- to understand */
+	SHOW_MODE,
+	ERROR_MODE	/* invalid user input */
+};
+
+/* what system power mode to use?  for now handle only standardized mode
+ * names; eventually when systems define their own state names, parse
+ * /sys/power/state
+ *
+ * "on" is used just to test the RTC alarm mechanism, bypassing all the
+ * wakeup-from-sleep infrastructure.  */
+static const char *mode_str[] = {
+	[STANDBY_MODE] = "standby",
+	[MEM_MODE] = "mem",
+	[FREEZE_MODE] = "freeze",
+	[DISK_MODE] = "disk",
+	[OFF_MODE] = "off",
+	[NO_MODE] = "no",
+	[ON_MODE] = "on",
+	[DISABLE_MODE] = "disable",
+	[SHOW_MODE] = "show"
+};
 
 enum clock_modes {
 	CM_AUTO,
@@ -243,7 +273,7 @@ static int setup_alarm(struct rtcwake_control *ctl, int fd, time_t *wakeup)
 	return 0;
 }
 
-static int is_suspend_available(const char *suspend)
+static int is_suspend_available(const int suspend)
 {
 	int rc;
 	char buf[32];
@@ -255,13 +285,13 @@ static int is_suspend_available(const char *suspend)
 	if (fgets(buf, sizeof buf, f) == NULL)
 		rc = -1;
 	else
-		rc = strstr(buf, suspend) != NULL;
+		rc = strstr(buf, mode_str[suspend]) != NULL;
 
 	fclose(f);
 	return rc;
 }
 
-static void suspend_system(struct rtcwake_control *ctl, const char *suspend)
+static void suspend_system(struct rtcwake_control *ctl, int suspend)
 {
 	FILE	*f = fopen(SYS_POWER_STATE_PATH, "w");
 
@@ -271,7 +301,7 @@ static void suspend_system(struct rtcwake_control *ctl, const char *suspend)
 	}
 
 	if (!ctl->dryrun) {
-		fprintf(f, "%s\n", suspend);
+		fprintf(f, "%s\n", mode_str[suspend]);
 		fflush(f);
 	}
 
@@ -372,6 +402,30 @@ static int print_alarm(struct rtcwake_control *ctl, int fd)
 	return 0;
 }
 
+static int get_mode(const char *optarg)
+{
+	if (!strcmp(optarg, mode_str[STANDBY_MODE]))
+		return STANDBY_MODE;
+	if (!strcmp(optarg, mode_str[MEM_MODE]))
+		return MEM_MODE;
+	if (!strcmp(optarg, mode_str[DISK_MODE]))
+		return DISK_MODE;
+	if (!strcmp(optarg, mode_str[ON_MODE]))
+		return ON_MODE;
+	if (!strcmp(optarg, mode_str[NO_MODE]))
+		return NO_MODE;
+	if (!strcmp(optarg, mode_str[OFF_MODE]))
+		return OFF_MODE;
+	if (!strcmp(optarg, mode_str[FREEZE_MODE]))
+		return FREEZE_MODE;
+	if (!strcmp(optarg, mode_str[DISABLE_MODE]))
+		return DISABLE_MODE;
+	if (!strcmp(optarg, mode_str[SHOW_MODE]))
+		return SHOW_MODE;
+	return ERROR_MODE;
+}
+
+
 int main(int argc, char **argv)
 {
 	struct rtcwake_control ctl = {
@@ -382,7 +436,7 @@ int main(int argc, char **argv)
 
 	char		*devname = DEFAULT_DEVICE;
 	unsigned	seconds = 0;
-	char		*suspend = DEFAULT_MODE;
+	int		suspend = STANDBY_MODE;
 
 	int		rc = EXIT_SUCCESS;
 	int		t;
@@ -417,8 +471,9 @@ int main(int argc, char **argv)
 			/* for better compatibility with hwclock */
 			ctl.adjfile = optarg;
 			break;
+
 		case 'a':
-			/* CM_AUTO is default */
+			ctl.clock_mode = CM_AUTO;
 			break;
 
 		case 'd':
@@ -429,31 +484,9 @@ int main(int argc, char **argv)
 			ctl.clock_mode = CM_LOCAL;
 			break;
 
-			/* what system power mode to use?  for now handle only
-			 * standardized mode names; eventually when systems
-			 * define their own state names, parse
-			 * /sys/power/state.
-			 *
-			 * "on" is used just to test the RTC alarm mechanism,
-			 * bypassing all the wakeup-from-sleep infrastructure.
-			 */
 		case 'm':
-			if (strcmp(optarg, "standby") == 0
-					|| strcmp(optarg, "mem") == 0
-					|| strcmp(optarg, "disk") == 0
-					|| strcmp(optarg, "on") == 0
-					|| strcmp(optarg, "no") == 0
-					|| strcmp(optarg, "off") == 0
-					|| strcmp(optarg, "freeze") == 0
-					|| strcmp(optarg, "disable") == 0
-					|| strcmp(optarg, "show") == 0
-			   ) {
-				suspend = optarg;
-				break;
-			}
-
-			errx(EXIT_FAILURE, _("unrecognized suspend state '%s'"),
-									optarg);
+			if ((suspend = get_mode(optarg)) == ERROR_MODE)
+				errx(EXIT_FAILURE, _("unrecognized suspend state '%s'"), optarg);
 			break;
 
 		case 'n':
@@ -502,9 +535,7 @@ int main(int argc, char **argv)
 		printf("%s",  ctl.clock_mode == CM_UTC ? _("Using UTC time.\n") :
 				_("Using local time.\n"));
 
-	if (!alarm && !seconds && strcmp(suspend,"disable") &&
-				  strcmp(suspend,"show")) {
-
+	if (!alarm && !seconds && suspend < DISABLE_MODE) {
 		warnx(_("must provide wake time (see -t and -s options)"));
 		usage(stderr);
 	}
@@ -520,8 +551,7 @@ int main(int argc, char **argv)
 		devname = new_devname;
 	}
 
-	if (strcmp(suspend, "on") != 0 && strcmp(suspend, "no") != 0
-			&& !is_wakeup_enabled(devname))
+	if (suspend != ON_MODE && suspend != NO_MODE && !is_wakeup_enabled(devname))
 		errx(EXIT_FAILURE, _("%s not enabled for wakeup events"), devname);
 
 	/* this RTC must exist and (if we'll sleep) be wakeup-enabled */
@@ -540,10 +570,10 @@ int main(int argc, char **argv)
 		printf(_("alarm %ld, sys_time %ld, rtc_time %ld, seconds %u\n"),
 				alarm, ctl.sys_time, ctl.rtc_time, seconds);
 
-	if (strcmp(suspend, "show") && strcmp(suspend, "disable")) {
-		if (strcmp(suspend, "no") && strcmp(suspend, "on") &&
-		    strcmp(suspend, "off") && is_suspend_available(suspend) <= 0) {
-			errx(EXIT_FAILURE, _("suspend to \"%s\" unavailable"), suspend);
+	if (suspend < DISABLE_MODE) {
+		if (suspend < OFF_MODE && is_suspend_available(suspend) <= 0) {
+			errx(EXIT_FAILURE, _("suspend to \"%s\" unavailable"),
+			     mode_str[suspend]);
 		}
 
 		/* care about alarm setup only if the show|disable
@@ -560,24 +590,24 @@ int main(int argc, char **argv)
 		if (setup_alarm(&ctl, fd, &alarm) < 0)
 			exit(EXIT_FAILURE);
 
-		if (strcmp(suspend, "no") == 0 || strcmp(suspend, "on") == 0)
+		if (suspend == NO_MODE || suspend == ON_MODE)
 			printf(_("%s: wakeup using %s at %s"),
 				program_invocation_short_name, devname,
 				ctime(&alarm));
 		else
 			printf(_("%s: wakeup from \"%s\" using %s at %s"),
-				program_invocation_short_name, suspend, devname,
+				program_invocation_short_name, mode_str[suspend], devname,
 				ctime(&alarm));
 		fflush(stdout);
 		xusleep(10 * 1000);
 	}
 
-	if (strcmp(suspend, "no") == 0) {
+	if (suspend == NO_MODE) {
 		if (ctl.verbose)
 			printf(_("suspend mode: no; leaving\n"));
 		ctl.dryrun = 1;	/* to skip disabling alarm at the end */
 
-	} else if (strcmp(suspend, "off") == 0) {
+	} else if (suspend == OFF_MODE) {
 		char *arg[5];
 		int i = 0;
 
@@ -597,7 +627,7 @@ int main(int argc, char **argv)
 			rc = EXIT_FAILURE;
 		}
 
-	} else if (strcmp(suspend, "on") == 0) {
+	} else if (suspend == ON_MODE) {
 		unsigned long data;
 
 		if (ctl.verbose)
@@ -615,12 +645,12 @@ int main(int argc, char **argv)
 			} while (!(data & RTC_AF));
 		}
 
-	} else if (strcmp(suspend, "disable") == 0) {
+	} else if (suspend == DISABLE_MODE) {
 		/* just break, alarm gets disabled in the end */
 		if (ctl.verbose)
 			printf(_("suspend mode: disable; disabling alarm\n"));
 
-	} else if(strcmp(suspend,"show") == 0) {
+	} else if (suspend == SHOW_MODE) {
 		if (ctl.verbose)
 			printf(_("suspend mode: show; printing alarm info\n"));
 		if (print_alarm(&ctl, fd))
@@ -629,7 +659,7 @@ int main(int argc, char **argv)
 
 	} else {
 		if (ctl.verbose)
-			printf(_("suspend mode: %s; suspending system\n"), suspend);
+			printf(_("suspend mode: %s; suspending system\n"), mode_str[suspend]);
 		sync();
 		suspend_system(&ctl, suspend);
 	}
-- 
2.3.0


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

* [PATCH 03/13] rtcwake: replace long if else statement with switch case
  2015-02-22 14:42 [PATCH 00/13] pull: rtcwake changes Sami Kerola
  2015-02-22 14:42 ` [PATCH 01/13] rtcwake: add rtcwake_control and remove global variables Sami Kerola
  2015-02-22 14:42 ` [PATCH 02/13] rtcwake: enumerate constant mode strings Sami Kerola
@ 2015-02-22 14:42 ` Sami Kerola
  2015-02-22 14:42 ` [PATCH 04/13] rtcwake: remove RTC_ALM_READ and RTC_ALM_SET compatibility Sami Kerola
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2015-02-22 14:42 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

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

diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
index 372e620..5f4b602 100644
--- a/sys-utils/rtcwake.c
+++ b/sys-utils/rtcwake.c
@@ -602,12 +602,15 @@ int main(int argc, char **argv)
 		xusleep(10 * 1000);
 	}
 
-	if (suspend == NO_MODE) {
+	switch (suspend) {
+	case NO_MODE:
 		if (ctl.verbose)
 			printf(_("suspend mode: no; leaving\n"));
 		ctl.dryrun = 1;	/* to skip disabling alarm at the end */
+		break;
 
-	} else if (suspend == OFF_MODE) {
+	case OFF_MODE:
+	{
 		char *arg[5];
 		int i = 0;
 
@@ -626,8 +629,10 @@ int main(int argc, char **argv)
 			warn(_("failed to execute %s"), _PATH_SHUTDOWN);
 			rc = EXIT_FAILURE;
 		}
-
-	} else if (suspend == ON_MODE) {
+		break;
+	}
+	case ON_MODE:
+	{
 		unsigned long data;
 
 		if (ctl.verbose)
@@ -644,20 +649,23 @@ int main(int argc, char **argv)
 					printf("... %s: %03lx\n", devname, data);
 			} while (!(data & RTC_AF));
 		}
-
-	} else if (suspend == DISABLE_MODE) {
+		break;
+	}
+	case DISABLE_MODE:
 		/* just break, alarm gets disabled in the end */
 		if (ctl.verbose)
 			printf(_("suspend mode: disable; disabling alarm\n"));
+		break;
 
-	} else if (suspend == SHOW_MODE) {
+	case SHOW_MODE:
 		if (ctl.verbose)
 			printf(_("suspend mode: show; printing alarm info\n"));
 		if (print_alarm(&ctl, fd))
 			rc = EXIT_FAILURE;
 		ctl.dryrun = 1;	/* don't really disable alarm in the end, just show */
+		break;
 
-	} else {
+	default:
 		if (ctl.verbose)
 			printf(_("suspend mode: %s; suspending system\n"), mode_str[suspend]);
 		sync();
-- 
2.3.0


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

* [PATCH 04/13] rtcwake: remove RTC_ALM_READ and RTC_ALM_SET compatibility
  2015-02-22 14:42 [PATCH 00/13] pull: rtcwake changes Sami Kerola
                   ` (2 preceding siblings ...)
  2015-02-22 14:42 ` [PATCH 03/13] rtcwake: replace long if else statement with switch case Sami Kerola
@ 2015-02-22 14:42 ` Sami Kerola
  2015-02-23 20:37   ` Benno Schulenberg
  2015-02-22 14:42 ` [PATCH 05/13] rtcwake: improve read_clock_mode() Sami Kerola
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Sami Kerola @ 2015-02-22 14:42 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

The RTC_WKALM_RD and RTC_WKALM_SET have avaRilable since 2.6.17, and
preferred way since 2007.  Keeping the availability to old interface is
no longer needed.

Reference: https://github.com/torvalds/linux/commit/e824290e5dcfaf2120da587b16d10dfdff8d5d3e
Reference: https://github.com/torvalds/linux/commit/f8245c26886c912627ebc49f714e4491261224c4
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/rtcwake.c | 42 ++++++------------------------------------
 1 file changed, 6 insertions(+), 36 deletions(-)

diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
index 5f4b602..a0e3151 100644
--- a/sys-utils/rtcwake.c
+++ b/sys-utils/rtcwake.c
@@ -250,24 +250,9 @@ static int setup_alarm(struct rtcwake_control *ctl, int fd, time_t *wakeup)
 
 	wake.enabled = 1;
 
-	/* First try the preferred RTC_WKALM_SET */
 	if (!ctl->dryrun && ioctl(fd, RTC_WKALM_SET, &wake) < 0) {
-		wake.enabled = 0;
-		/* Fall back on the non-preferred way of setting wakeups; only
-		* works for alarms < 24 hours from now */
-		if ((ctl->rtc_time + (24 * 60 * 60)) > *wakeup) {
-			if (ioctl(fd, RTC_ALM_SET, &wake.time) < 0) {
-				warn(_("set rtc alarm failed"));
-				return -1;
-			}
-			if (ioctl(fd, RTC_AIE_ON, 0) < 0) {
-				warn(_("enable rtc alarm failed"));
-				return -1;
-			}
-		} else {
-			warn(_("set rtc wake alarm failed"));
-			return -1;
-		}
+		warn(_("set rtc wake alarm failed"));
+		return -1;
 	}
 
 	return 0;
@@ -358,19 +343,9 @@ static int print_alarm(struct rtcwake_control *ctl, int fd)
 	struct tm tm;
 	time_t alarm;
 
-	 /* First try the preferred RTC_WKALM_RD */
 	if (ioctl(fd, RTC_WKALM_RD, &wake) < 0) {
-		/* Fall back on the non-preferred way of reading wakeups; only
-		 * works for alarms < 24 hours from now
-		 *
-		 * set wake.enabled to 1 and determine from value of the year-1
-		 * means disabled
-		 */
-		wake.enabled = 1;
-		if (ioctl(fd, RTC_ALM_READ, &wake.time) < 0) {
-			warn(_("read rtc alarm failed"));
-			return -1;
-		}
+		warn(_("read rtc alarm failed"));
+		return -1;
 	}
 
 	if (wake.enabled != 1 || wake.time.tm_year == -1) {
@@ -673,16 +648,11 @@ int main(int argc, char **argv)
 	}
 
 	if (!ctl.dryrun) {
-		/* try to disable the alarm with the preferred RTC_WKALM_RD and
-		 * RTC_WKALM_SET calls, if it fails fall back to RTC_AIE_OFF
-		 */
 		struct rtc_wkalrm wake;
 
 		if (ioctl(fd, RTC_WKALM_RD, &wake) < 0) {
-			if (ioctl(fd, RTC_AIE_OFF, 0) < 0) {
-				warn(_("disable rtc alarm interrupt failed"));
-				rc = EXIT_FAILURE;
-			}
+			warn(_("read rtc alarm failed"));
+			rc = EXIT_FAILURE;
 		} else {
 			wake.enabled = 0;
 			if (ioctl(fd, RTC_WKALM_SET, &wake) < 0) {
-- 
2.3.0


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

* [PATCH 05/13] rtcwake: improve read_clock_mode()
  2015-02-22 14:42 [PATCH 00/13] pull: rtcwake changes Sami Kerola
                   ` (3 preceding siblings ...)
  2015-02-22 14:42 ` [PATCH 04/13] rtcwake: remove RTC_ALM_READ and RTC_ALM_SET compatibility Sami Kerola
@ 2015-02-22 14:42 ` Sami Kerola
  2015-02-22 14:42 ` [PATCH 06/13] rtcwake: add human readable --date timestamp format Sami Kerola
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2015-02-22 14:42 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

Make skipping two lines more robust, and add message about unexpected
adjfile contents when running with --verbose.

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

diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
index a0e3151..790863f 100644
--- a/sys-utils/rtcwake.c
+++ b/sys-utils/rtcwake.c
@@ -47,8 +47,6 @@
 #define	RTC_AF	0x20
 #define	RTC_UF	0x10
 
-#define MAX_LINE		1024
-
 #define RTC_PATH		"/sys/class/rtc/%s/device/power/wakeup"
 #define SYS_POWER_STATE_PATH	"/sys/power/state"
 #define DEFAULT_DEVICE		"/dev/rtc0"
@@ -299,26 +297,25 @@ static void suspend_system(struct rtcwake_control *ctl, int suspend)
 static int read_clock_mode(struct rtcwake_control *ctl)
 {
 	FILE *fp;
-	char linebuf[MAX_LINE];
+	char ch, linebuf[8];
+	int lines = 2, give_up = 0xffff;
 
 	fp = fopen(ctl->adjfile, "r");
 	if (!fp)
 		return -1;
 
-	/* skip first line */
-	if (!fgets(linebuf, MAX_LINE, fp)) {
-		fclose(fp);
-		return -1;
-	}
-
-	/* skip second line */
-	if (!fgets(linebuf, MAX_LINE, fp)) {
-		fclose(fp);
-		return -1;
-	}
+	/* skip lines */
+	do {
+		if ((ch = fgetc(fp)) == EOF || 0 < give_up--) {
+			fclose(fp);
+			return -1;
+		}
+		if (ch == '\n' && --lines == 0)
+			break;
+	} while (1);
 
 	/* read third line */
-	if (!fgets(linebuf, MAX_LINE, fp)) {
+	if (!fgets(linebuf, sizeof linebuf, fp)) {
 		fclose(fp);
 		return -1;
 	}
@@ -327,6 +324,8 @@ static int read_clock_mode(struct rtcwake_control *ctl)
 		ctl->clock_mode = CM_UTC;
 	else if (strncmp(linebuf, "LOCAL", 5) == 0)
 		ctl->clock_mode = CM_LOCAL;
+	else if (ctl->verbose)
+		warnx(_("unexpected third line in: %s: %s"), ctl->adjfile, linebuf);
 
 	fclose(fp);
 
-- 
2.3.0


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

* [PATCH 06/13] rtcwake: add human readable --date timestamp format
  2015-02-22 14:42 [PATCH 00/13] pull: rtcwake changes Sami Kerola
                   ` (4 preceding siblings ...)
  2015-02-22 14:42 ` [PATCH 05/13] rtcwake: improve read_clock_mode() Sami Kerola
@ 2015-02-22 14:42 ` Sami Kerola
  2015-02-23 20:32   ` Benno Schulenberg
  2015-02-22 14:42 ` [PATCH 07/13] rtcwake: fix preprocessor redefinition Sami Kerola
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Sami Kerola @ 2015-02-22 14:42 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 bash-completion/rtcwake |  7 ++++++-
 sys-utils/rtcwake.8.in  | 19 ++++++++++++++++++-
 sys-utils/rtcwake.c     | 24 +++++++++++++++++++-----
 3 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/bash-completion/rtcwake b/bash-completion/rtcwake
index d7d1427..51566a2 100644
--- a/bash-completion/rtcwake
+++ b/bash-completion/rtcwake
@@ -23,11 +23,16 @@ _rtcwake_module()
 			COMPREPLY=( $(compgen -W "time_t" -- $cur) )
 			return 0
 			;;
+		'--date')
+			COMPREPLY=( $(compgen -W "YYYYMMDDhhmmss" -- $cur) )
+			return 0
+			;;
 		'-h'|'--help'|'-V'|'--version')
 			return 0
 			;;
 	esac
-	OPTS="--device
+	OPTS="	--date
+		--device
 		--dry-run
 		--local
 		--mode
diff --git a/sys-utils/rtcwake.8.in b/sys-utils/rtcwake.8.in
index 5ec9c6c..d81f9e1 100644
--- a/sys-utils/rtcwake.8.in
+++ b/sys-utils/rtcwake.8.in
@@ -120,6 +120,23 @@ is the time in seconds since 1970-01-01, 00:00 UTC.  Use the
 .BR date (1)
 tool to convert between human-readable time and \fItime_t\fP.
 .TP
+.BR \-\-date " \fItimestamp"
+Set the wakeup time to the value of the timestamp.  Format of the
+timestmap can be any of the following:
+.TS
+tab(|);
+left l2 l.
+YYYYMMDDhhmmss
+YYYY-MM-DD hh:mm:ss
+YYYY-MM-DD hh:mm|(seconds will be set to 00)
+YYYY-MM-DD|(time will be set to 00:00:00)
+hh:mm:ss|(date will be set to today)
+hh:mm|(date will be set to today, seconds to 00)
+today|(time is set to 00:00:00)
+tomorrow|(time is set to 00:00:00)
++5min
+.TE
+.TP
 .BR \-u , " \-\-utc"
 Assume that the hardware clock is set to UTC (Universal Time Coordinated),
 regardless of the contents of the \fIadjtime\fP file.
@@ -156,4 +173,4 @@ There is NO WARRANTY, to the extent permitted by law.
 The rtcwake command is part of the util-linux package and is available from the
 .UR ftp://\:ftp.kernel.org\:/pub\:/linux\:/utils\:/util-linux/
 Linux Kernel Archive
-.UE .
\ No newline at end of file
+.UE .
diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
index 790863f..4aa4fd3 100644
--- a/sys-utils/rtcwake.c
+++ b/sys-utils/rtcwake.c
@@ -41,6 +41,7 @@
 #include "strutils.h"
 #include "c.h"
 #include "closestream.h"
+#include "timeutils.h"
 
 /* constants from legacy PC/AT hardware */
 #define	RTC_PF	0x40
@@ -118,6 +119,7 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
 	fputs(_(" -m, --mode <mode>        standby|mem|... sleep mode\n"), out);
 	fputs(_(" -s, --seconds <seconds>  seconds to sleep\n"), out);
 	fputs(_(" -t, --time <time_t>      time to wake\n"), out);
+	fputs(_("     --date <timestamp>   date time of timestamp to wake\n"), out);
 	fputs(_(" -u, --utc                RTC uses UTC\n"), out);
 	fputs(_(" -v, --verbose            verbose messages\n"), out);
 
@@ -417,6 +419,10 @@ int main(int argc, char **argv)
 	int		fd;
 	time_t		alarm = 0;
 
+	enum {
+		OPT_DATE = CHAR_MAX + 1
+	};
+
 	static const struct option long_options[] = {
 		{"adjfile",     required_argument,      0, 'A'},
 		{"auto",	no_argument,		0, 'a'},
@@ -430,6 +436,7 @@ int main(int argc, char **argv)
 		{"device",	required_argument,	0, 'd'},
 		{"seconds",	required_argument,	0, 's'},
 		{"time",	required_argument,	0, 't'},
+		{"date",	required_argument,	0, OPT_DATE},
 		{0,		0,			0, 0  }
 	};
 
@@ -467,18 +474,25 @@ int main(int argc, char **argv)
 			ctl.dryrun = 1;
 			break;
 
-			/* alarm time, seconds-to-sleep (relative) */
 		case 's':
+			/* alarm time, seconds-to-sleep (relative) */
 			seconds = strtou32_or_err(optarg, _("invalid seconds argument"));
 			break;
 
-			/* alarm time, time_t (absolute, seconds since
-			 * 1/1 1970 UTC)
-			 */
 		case 't':
+			/* alarm time, time_t (absolute, seconds since epoc) */
 			alarm = strtou32_or_err(optarg, _("invalid time argument"));
 			break;
 
+		case OPT_DATE:
+		{
+			/* alarm time, see timestamp format from manual */
+			usec_t p;
+			if (parse_timestamp(optarg, &p) < 0)
+				errx(EXIT_FAILURE, _("invalid time value \"%s\""), optarg);
+			alarm = (time_t) (p / 1000000);
+			break;
+		}
 		case 'u':
 			ctl.clock_mode = CM_UTC;
 			break;
@@ -510,7 +524,7 @@ int main(int argc, char **argv)
 				_("Using local time.\n"));
 
 	if (!alarm && !seconds && suspend < DISABLE_MODE) {
-		warnx(_("must provide wake time (see -t and -s options)"));
+		warnx(_("must provide wake time (see --seconds, --time, and --date options)"));
 		usage(stderr);
 	}
 
-- 
2.3.0


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

* [PATCH 07/13] rtcwake: fix preprocessor redefinition
  2015-02-22 14:42 [PATCH 00/13] pull: rtcwake changes Sami Kerola
                   ` (5 preceding siblings ...)
  2015-02-22 14:42 ` [PATCH 06/13] rtcwake: add human readable --date timestamp format Sami Kerola
@ 2015-02-22 14:42 ` Sami Kerola
  2015-02-22 14:42 ` [PATCH 08/13] rtcwake: clean up struct tm initializations Sami Kerola
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2015-02-22 14:42 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

The RTC_AF is expected to be part of linux/rtc.h file.

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

diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
index 4aa4fd3..c3e0eb6 100644
--- a/sys-utils/rtcwake.c
+++ b/sys-utils/rtcwake.c
@@ -43,10 +43,9 @@
 #include "closestream.h"
 #include "timeutils.h"
 
-/* constants from legacy PC/AT hardware */
-#define	RTC_PF	0x40
-#define	RTC_AF	0x20
-#define	RTC_UF	0x10
+#ifndef RTC_AF
+# define	RTC_AF	0x20	/* Alarm interrupt */
+#endif
 
 #define RTC_PATH		"/sys/class/rtc/%s/device/power/wakeup"
 #define SYS_POWER_STATE_PATH	"/sys/power/state"
-- 
2.3.0


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

* [PATCH 08/13] rtcwake: clean up struct tm initializations
  2015-02-22 14:42 [PATCH 00/13] pull: rtcwake changes Sami Kerola
                   ` (6 preceding siblings ...)
  2015-02-22 14:42 ` [PATCH 07/13] rtcwake: fix preprocessor redefinition Sami Kerola
@ 2015-02-22 14:42 ` Sami Kerola
  2015-02-22 14:42 ` [PATCH 09/13] rtcwake: do not overwrite device name Sami Kerola
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2015-02-22 14:42 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

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

diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
index c3e0eb6..f951c9c 100644
--- a/sys-utils/rtcwake.c
+++ b/sys-utils/rtcwake.c
@@ -159,7 +159,7 @@ static int is_wakeup_enabled(const char *devname)
 
 static int get_basetimes(struct rtcwake_control *ctl, int fd)
 {
-	struct tm	tm;
+	struct tm	tm = { 0 };
 	struct rtc_time	rtc;
 
 	/* this process works in RTC time, except when working
@@ -185,7 +185,6 @@ static int get_basetimes(struct rtcwake_control *ctl, int fd)
 	/* convert rtc_time to normal arithmetic-friendly form,
 	 * updating tm.tm_wday as used by asctime().
 	 */
-	memset(&tm, 0, sizeof tm);
 	tm.tm_sec = rtc.tm_sec;
 	tm.tm_min = rtc.tm_min;
 	tm.tm_hour = rtc.tm_hour;
@@ -339,8 +338,7 @@ static int read_clock_mode(struct rtcwake_control *ctl)
 static int print_alarm(struct rtcwake_control *ctl, int fd)
 {
 	struct rtc_wkalrm wake;
-	struct rtc_time rtc;
-	struct tm tm;
+	struct tm tm = { 0 };
 	time_t alarm;
 
 	if (ioctl(fd, RTC_WKALM_RD, &wake) < 0) {
@@ -353,15 +351,12 @@ static int print_alarm(struct rtcwake_control *ctl, int fd)
 		return 0;
 	}
 
-	rtc = wake.time;
-
-	memset(&tm, 0, sizeof tm);
-	tm.tm_sec = rtc.tm_sec;
-	tm.tm_min = rtc.tm_min;
-	tm.tm_hour = rtc.tm_hour;
-	tm.tm_mday = rtc.tm_mday;
-	tm.tm_mon = rtc.tm_mon;
-	tm.tm_year = rtc.tm_year;
+	tm.tm_sec = wake.time.tm_sec;
+	tm.tm_min = wake.time.tm_min;
+	tm.tm_hour = wake.time.tm_hour;
+	tm.tm_mday = wake.time.tm_mday;
+	tm.tm_mon = wake.time.tm_mon;
+	tm.tm_year = wake.time.tm_year;
 	tm.tm_isdst = -1;  /* assume the system knows better than the RTC */
 
 	alarm = mktime(&tm);
-- 
2.3.0


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

* [PATCH 09/13] rtcwake: do not overwrite device name
  2015-02-22 14:42 [PATCH 00/13] pull: rtcwake changes Sami Kerola
                   ` (7 preceding siblings ...)
  2015-02-22 14:42 ` [PATCH 08/13] rtcwake: clean up struct tm initializations Sami Kerola
@ 2015-02-22 14:42 ` Sami Kerola
  2015-02-24 13:00   ` Karel Zak
  2015-02-22 14:42 ` [PATCH 10/13] bash-completion: add freeze mode to rtcwake Sami Kerola
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Sami Kerola @ 2015-02-22 14:42 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

This improves error messaging, and allows freeing path to device when it
is not needed.

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

diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
index f951c9c..7f1e351 100644
--- a/sys-utils/rtcwake.c
+++ b/sys-utils/rtcwake.c
@@ -49,7 +49,7 @@
 
 #define RTC_PATH		"/sys/class/rtc/%s/device/power/wakeup"
 #define SYS_POWER_STATE_PATH	"/sys/power/state"
-#define DEFAULT_DEVICE		"/dev/rtc0"
+#define DEFAULT_DEVICE		"rtc0"
 
 enum rtc_modes {	/* manual page --mode option explains these. */
 	STANDBY_MODE = 0,
@@ -137,7 +137,7 @@ static int is_wakeup_enabled(const char *devname)
 	FILE	*f;
 
 	/* strip the '/dev/' from the devname here */
-	snprintf(buf, sizeof buf, RTC_PATH, devname + strlen("/dev/"));
+	snprintf(buf, sizeof buf, RTC_PATH, devname);
 	f = fopen(buf, "r");
 	if (!f) {
 		warn(_("cannot open %s"), buf);
@@ -395,6 +395,25 @@ static int get_mode(const char *optarg)
 	return ERROR_MODE;
 }
 
+static int open_dev_rtc(const char *devname)
+{
+	int fd;
+	char *devpath;
+	size_t len = strlen(devname);
+
+	devpath = xmalloc(sizeof "/dev/" + len);
+	memcpy(devpath, "/dev/", sizeof "/dev/");
+	memcpy(devpath + strlen("/dev/"), devname, len + 1);
+#ifdef O_CLOEXEC
+	fd = open(devpath, O_RDONLY | O_CLOEXEC);
+#else
+	fd = open(devpath, O_RDONLY);
+#endif
+	if (fd < 0)
+		err(EXIT_FAILURE, _("%s: unable to find device"), devpath);
+	free(devpath);
+	return fd;
+}
 
 int main(int argc, char **argv)
 {
@@ -522,29 +541,12 @@ int main(int argc, char **argv)
 		usage(stderr);
 	}
 
-	/* when devname doesn't start with /dev, append it */
-	if (strncmp(devname, "/dev/", strlen("/dev/")) != 0) {
-		char *new_devname;
-
-		new_devname = xmalloc(strlen(devname) + strlen("/dev/") + 1);
-
-		strcpy(new_devname, "/dev/");
-		strcat(new_devname, devname);
-		devname = new_devname;
-	}
+	/* device must exist and (if we'll sleep) be wakeup-enabled */
+	fd = open_dev_rtc(devname);
 
 	if (suspend != ON_MODE && suspend != NO_MODE && !is_wakeup_enabled(devname))
 		errx(EXIT_FAILURE, _("%s not enabled for wakeup events"), devname);
 
-	/* this RTC must exist and (if we'll sleep) be wakeup-enabled */
-#ifdef O_CLOEXEC
-	fd = open(devname, O_RDONLY | O_CLOEXEC);
-#else
-	fd = open(devname, O_RDONLY);
-#endif
-	if (fd < 0)
-		err(EXIT_FAILURE, _("cannot open %s"), devname);
-
 	/* relative or absolute alarm time, normalized to time_t */
 	if (get_basetimes(&ctl, fd) < 0)
 		exit(EXIT_FAILURE);
-- 
2.3.0


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

* [PATCH 10/13] bash-completion: add freeze mode to rtcwake
  2015-02-22 14:42 [PATCH 00/13] pull: rtcwake changes Sami Kerola
                   ` (8 preceding siblings ...)
  2015-02-22 14:42 ` [PATCH 09/13] rtcwake: do not overwrite device name Sami Kerola
@ 2015-02-22 14:42 ` Sami Kerola
  2015-02-24 13:01   ` Karel Zak
  2015-02-22 14:42 ` [PATCH 11/13] rtcwake: improve coding style Sami Kerola
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Sami Kerola @ 2015-02-22 14:42 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

Missed when commit ece44f19f423408f576f348fed2845c876d72c6e added support
to freeze.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 bash-completion/rtcwake | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bash-completion/rtcwake b/bash-completion/rtcwake
index 51566a2..70ae743 100644
--- a/bash-completion/rtcwake
+++ b/bash-completion/rtcwake
@@ -12,7 +12,7 @@ _rtcwake_module()
 			return 0
 			;;
 		'-m'|'--mode')
-			COMPREPLY=( $(compgen -W "standby mem disk off no on disable show" -- $cur) )
+			COMPREPLY=( $(compgen -W "standby freeze mem disk off no on disable show" -- $cur) )
 			return 0
 			;;
 		'-s'|'--seconds')
-- 
2.3.0


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

* [PATCH 11/13] rtcwake: improve coding style
  2015-02-22 14:42 [PATCH 00/13] pull: rtcwake changes Sami Kerola
                   ` (9 preceding siblings ...)
  2015-02-22 14:42 ` [PATCH 10/13] bash-completion: add freeze mode to rtcwake Sami Kerola
@ 2015-02-22 14:42 ` Sami Kerola
  2015-02-23 20:40   ` Benno Schulenberg
  2015-02-22 14:42 ` [PATCH 12/13] rtcwake: make some command line options mutually exclusive Sami Kerola
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Sami Kerola @ 2015-02-22 14:42 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

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

diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
index 7f1e351..e6f2170 100644
--- a/sys-utils/rtcwake.c
+++ b/sys-utils/rtcwake.c
@@ -19,29 +19,27 @@
  * That flag should not be needed on systems with adjtime support.
  */
 
-#include <stdio.h>
-#include <getopt.h>
+#include <errno.h>
 #include <fcntl.h>
+#include <getopt.h>
 #include <libgen.h>
+#include <linux/rtc.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <unistd.h>
-#include <errno.h>
-#include <time.h>
-
 #include <sys/ioctl.h>
 #include <sys/time.h>
 #include <sys/types.h>
+#include <time.h>
+#include <unistd.h>
 
-#include <linux/rtc.h>
-
+#include "c.h"
+#include "closestream.h"
 #include "nls.h"
-#include "xalloc.h"
 #include "pathnames.h"
 #include "strutils.h"
-#include "c.h"
-#include "closestream.h"
 #include "timeutils.h"
+#include "xalloc.h"
 
 #ifndef RTC_AF
 # define	RTC_AF	0x20	/* Alarm interrupt */
@@ -147,12 +145,10 @@ static int is_wakeup_enabled(const char *devname)
 	fclose(f);
 	if (!s)
 		return 0;
-
 	s = strchr(buf, '\n');
 	if (!s)
 		return 0;
 	*s = 0;
-
 	/* wakeup events could be disabled or not supported */
 	return strcmp(buf, "enabled") == 0;
 }
@@ -162,16 +158,13 @@ static int get_basetimes(struct rtcwake_control *ctl, int fd)
 	struct tm	tm = { 0 };
 	struct rtc_time	rtc;
 
-	/* this process works in RTC time, except when working
-	 * with the system clock (which always uses UTC).
-	 */
+	/* this process works in RTC time, except when working with the
+	 * system clock (which always uses UTC). */
 	if (ctl->clock_mode == CM_UTC)
 		setenv("TZ", "UTC", 1);
 	tzset();
-
-	/* read rtc and system clocks "at the same time", or as
-	 * precisely (+/- a second) as we can read them.
-	 */
+	/* read rtc and system clocks "at the same time", or as precisely
+	 * (+/- a second) as we can read them. */
 	if (ioctl(fd, RTC_RD_TIME, &rtc) < 0) {
 		warn(_("read rtc time failed"));
 		return -1;
@@ -181,10 +174,8 @@ static int get_basetimes(struct rtcwake_control *ctl, int fd)
 		warn(_("read system time failed"));
 		return -1;
 	}
-
-	/* convert rtc_time to normal arithmetic-friendly form,
-	 * updating tm.tm_wday as used by asctime().
-	 */
+	/* convert rtc_time to normal arithmetic-friendly form, updating
+	 * tm.tm_wday as used by asctime(). */
 	tm.tm_sec = rtc.tm_sec;
 	tm.tm_min = rtc.tm_min;
 	tm.tm_hour = rtc.tm_hour;
@@ -193,20 +184,16 @@ static int get_basetimes(struct rtcwake_control *ctl, int fd)
 	tm.tm_year = rtc.tm_year;
 	tm.tm_isdst = -1;  /* assume the system knows better than the RTC */
 	ctl->rtc_time = mktime(&tm);
-
 	if (ctl->rtc_time == (time_t)-1) {
 		warn(_("convert rtc time failed"));
 		return -1;
 	}
-
 	if (ctl->verbose) {
 		/* Unless the system uses UTC, either delta or tzone
 		 * reflects a seconds offset from UTC.  The value can
-		 * help sort out problems like bugs in your C library.
-		 */
+		 * help sort out problems like bugs in your C library. */
 		printf("\tdelta   = %ld\n", ctl->sys_time - ctl->rtc_time);
 		printf("\ttzone   = %ld\n", timezone);
-
 		printf("\ttzname  = %s\n", tzname[daylight]);
 		gmtime_r(&ctl->rtc_time, &tm);
 		printf("\tsystime = %ld, (UTC) %s",
@@ -214,7 +201,6 @@ static int get_basetimes(struct rtcwake_control *ctl, int fd)
 		printf("\trtctime = %ld, (UTC) %s",
 				(long) ctl->rtc_time, asctime(&tm));
 	}
-
 	return 0;
 }
 
@@ -223,36 +209,31 @@ static int setup_alarm(struct rtcwake_control *ctl, int fd, time_t *wakeup)
 	struct tm		*tm;
 	struct rtc_wkalrm	wake;
 
-	/* The wakeup time is in POSIX time (more or less UTC).
-	 * Ideally RTCs use that same time; but PCs can't do that
-	 * if they need to boot MS-Windows.  Messy...
+	/* The wakeup time is in POSIX time (more or less UTC).  Ideally
+	 * RTCs use that same time; but PCs can't do that if they need to
+	 * boot MS-Windows.  Messy...
 	 *
-	 * When clock_mode == CM_UTC this process's timezone is UTC,
-	 * so we'll pass a UTC date to the RTC.
+	 * When clock_mode == CM_UTC this process's timezone is UTC, so
+	 * we'll pass a UTC date to the RTC.
 	 *
-	 * Else clock_mode == CM_LOCAL so the time given to the RTC
-	 * will instead use the local time zone.
-	 */
+	 * Else clock_mode == CM_LOCAL so the time given to the RTC will
+	 * instead use the local time zone. */
 	tm = localtime(wakeup);
-
 	wake.time.tm_sec = tm->tm_sec;
 	wake.time.tm_min = tm->tm_min;
 	wake.time.tm_hour = tm->tm_hour;
 	wake.time.tm_mday = tm->tm_mday;
 	wake.time.tm_mon = tm->tm_mon;
 	wake.time.tm_year = tm->tm_year;
-	/* wday, yday, and isdst fields are unused by Linux */
+	/* wday, yday, and isdst fields are unused */
 	wake.time.tm_wday = -1;
 	wake.time.tm_yday = -1;
 	wake.time.tm_isdst = -1;
-
 	wake.enabled = 1;
-
 	if (!ctl->dryrun && ioctl(fd, RTC_WKALM_SET, &wake) < 0) {
 		warn(_("set rtc wake alarm failed"));
 		return -1;
 	}
-
 	return 0;
 }
 
@@ -264,12 +245,10 @@ static int is_suspend_available(const int suspend)
 
 	if (!f)
 		return -1;
-
 	if (fgets(buf, sizeof buf, f) == NULL)
 		rc = -1;
 	else
 		rc = strstr(buf, mode_str[suspend]) != NULL;
-
 	fclose(f);
 	return rc;
 }
@@ -282,18 +261,15 @@ static void suspend_system(struct rtcwake_control *ctl, int suspend)
 		warn(_("cannot open %s"), SYS_POWER_STATE_PATH);
 		return;
 	}
-
 	if (!ctl->dryrun) {
 		fprintf(f, "%s\n", mode_str[suspend]);
 		fflush(f);
 	}
-
 	/* this executes after wake from suspend */
 	if (close_stream(f))
 		errx(EXIT_FAILURE, _("write error"));
 }
 
-
 static int read_clock_mode(struct rtcwake_control *ctl)
 {
 	FILE *fp;
@@ -303,7 +279,6 @@ static int read_clock_mode(struct rtcwake_control *ctl)
 	fp = fopen(ctl->adjfile, "r");
 	if (!fp)
 		return -1;
-
 	/* skip lines */
 	do {
 		if ((ch = fgetc(fp)) == EOF || 0 < give_up--) {
@@ -313,28 +288,21 @@ static int read_clock_mode(struct rtcwake_control *ctl)
 		if (ch == '\n' && --lines == 0)
 			break;
 	} while (1);
-
 	/* read third line */
 	if (!fgets(linebuf, sizeof linebuf, fp)) {
 		fclose(fp);
 		return -1;
 	}
-
 	if (strncmp(linebuf, "UTC", 3) == 0)
 		ctl->clock_mode = CM_UTC;
 	else if (strncmp(linebuf, "LOCAL", 5) == 0)
 		ctl->clock_mode = CM_LOCAL;
 	else if (ctl->verbose)
 		warnx(_("unexpected third line in: %s: %s"), ctl->adjfile, linebuf);
-
 	fclose(fp);
-
 	return 0;
 }
 
-/**
- * print basic alarm settings
- */
 static int print_alarm(struct rtcwake_control *ctl, int fd)
 {
 	struct rtc_wkalrm wake;
@@ -345,12 +313,10 @@ static int print_alarm(struct rtcwake_control *ctl, int fd)
 		warn(_("read rtc alarm failed"));
 		return -1;
 	}
-
 	if (wake.enabled != 1 || wake.time.tm_year == -1) {
 		printf(_("alarm: off\n"));
 		return 0;
 	}
-
 	tm.tm_sec = wake.time.tm_sec;
 	tm.tm_min = wake.time.tm_min;
 	tm.tm_hour = wake.time.tm_hour;
@@ -358,16 +324,13 @@ static int print_alarm(struct rtcwake_control *ctl, int fd)
 	tm.tm_mon = wake.time.tm_mon;
 	tm.tm_year = wake.time.tm_year;
 	tm.tm_isdst = -1;  /* assume the system knows better than the RTC */
-
 	alarm = mktime(&tm);
 	if (alarm == (time_t)-1) {
 		warn(_("convert time failed"));
 		return -1;
 	}
-
 	/* 0 if both UTC, or expresses diff if RTC in local time */
 	alarm += ctl->sys_time - ctl->rtc_time;
-
 	printf(_("alarm: on  %s"), ctime(&alarm));
 	return 0;
 }
@@ -422,20 +385,16 @@ int main(int argc, char **argv)
 		.clock_mode = CM_AUTO,
 		0
 	};
-
-	char		*devname = DEFAULT_DEVICE;
-	unsigned	seconds = 0;
-	int		suspend = STANDBY_MODE;
-
-	int		rc = EXIT_SUCCESS;
-	int		t;
-	int		fd;
-	time_t		alarm = 0;
-
+	char *devname = DEFAULT_DEVICE;
+	unsigned seconds = 0;
+	int suspend = STANDBY_MODE;
+	int rc = EXIT_SUCCESS;
+	int t;
+	int fd;
+	time_t alarm = 0;
 	enum {
 		OPT_DATE = CHAR_MAX + 1
 	};
-
 	static const struct option long_options[] = {
 		{"adjfile",     required_argument,      0, 'A'},
 		{"auto",	no_argument,		0, 'a'},
@@ -457,7 +416,6 @@ int main(int argc, char **argv)
 	bindtextdomain(PACKAGE, LOCALEDIR);
 	textdomain(PACKAGE);
 	atexit(close_stdout);
-
 	while ((t = getopt_long(argc, argv, "A:ahd:lm:ns:t:uVv",
 					long_options, NULL)) != EOF) {
 		switch (t) {
@@ -465,41 +423,32 @@ int main(int argc, char **argv)
 			/* for better compatibility with hwclock */
 			ctl.adjfile = optarg;
 			break;
-
 		case 'a':
 			ctl.clock_mode = CM_AUTO;
 			break;
-
 		case 'd':
 			devname = optarg;
 			break;
-
 		case 'l':
 			ctl.clock_mode = CM_LOCAL;
 			break;
-
 		case 'm':
 			if ((suspend = get_mode(optarg)) == ERROR_MODE)
 				errx(EXIT_FAILURE, _("unrecognized suspend state '%s'"), optarg);
 			break;
-
 		case 'n':
 			ctl.dryrun = 1;
 			break;
-
 		case 's':
 			/* alarm time, seconds-to-sleep (relative) */
 			seconds = strtou32_or_err(optarg, _("invalid seconds argument"));
 			break;
-
 		case 't':
 			/* alarm time, time_t (absolute, seconds since epoc) */
 			alarm = strtou32_or_err(optarg, _("invalid time argument"));
 			break;
-
 		case OPT_DATE:
-		{
-			/* alarm time, see timestamp format from manual */
+		{	/* alarm time, see timestamp format from manual */
 			usec_t p;
 			if (parse_timestamp(optarg, &p) < 0)
 				errx(EXIT_FAILURE, _("invalid time value \"%s\""), optarg);
@@ -509,22 +458,18 @@ int main(int argc, char **argv)
 		case 'u':
 			ctl.clock_mode = CM_UTC;
 			break;
-
 		case 'v':
 			ctl.verbose = 1;
 			break;
-
 		case 'V':
 			printf(UTIL_LINUX_VERSION);
 			exit(EXIT_SUCCESS);
-
 		case 'h':
 			usage(stdout);
 		default:
 			usage(stderr);
 		}
 	}
-
 	if (ctl.clock_mode == CM_AUTO) {
 		if (read_clock_mode(&ctl) < 0) {
 			printf(_("%s: assuming RTC uses UTC ...\n"),
@@ -535,34 +480,27 @@ int main(int argc, char **argv)
 	if (ctl.verbose)
 		printf("%s",  ctl.clock_mode == CM_UTC ? _("Using UTC time.\n") :
 				_("Using local time.\n"));
-
 	if (!alarm && !seconds && suspend < DISABLE_MODE) {
 		warnx(_("must provide wake time (see --seconds, --time, and --date options)"));
 		usage(stderr);
 	}
-
 	/* device must exist and (if we'll sleep) be wakeup-enabled */
 	fd = open_dev_rtc(devname);
-
 	if (suspend != ON_MODE && suspend != NO_MODE && !is_wakeup_enabled(devname))
 		errx(EXIT_FAILURE, _("%s not enabled for wakeup events"), devname);
-
 	/* relative or absolute alarm time, normalized to time_t */
 	if (get_basetimes(&ctl, fd) < 0)
 		exit(EXIT_FAILURE);
 	if (ctl.verbose)
 		printf(_("alarm %ld, sys_time %ld, rtc_time %ld, seconds %u\n"),
 				alarm, ctl.sys_time, ctl.rtc_time, seconds);
-
 	if (suspend < DISABLE_MODE) {
 		if (suspend < OFF_MODE && is_suspend_available(suspend) <= 0) {
 			errx(EXIT_FAILURE, _("suspend to \"%s\" unavailable"),
 			     mode_str[suspend]);
 		}
-
-		/* care about alarm setup only if the show|disable
-		 * modes are not set
-		 */
+		/* care about alarm setup only if the show or disable
+		 * modes are not set */
 		if (alarm) {
 			if (alarm < ctl.sys_time)
 				errx(EXIT_FAILURE, _("time doesn't go backward to %s"),
@@ -570,10 +508,8 @@ int main(int argc, char **argv)
 			alarm += ctl.sys_time - ctl.rtc_time;
 		} else
 			alarm = ctl.rtc_time + seconds + 1;
-
 		if (setup_alarm(&ctl, fd, &alarm) < 0)
 			exit(EXIT_FAILURE);
-
 		if (suspend == NO_MODE || suspend == ON_MODE)
 			printf(_("%s: wakeup using %s at %s"),
 				program_invocation_short_name, devname,
@@ -585,14 +521,12 @@ int main(int argc, char **argv)
 		fflush(stdout);
 		xusleep(10 * 1000);
 	}
-
 	switch (suspend) {
 	case NO_MODE:
 		if (ctl.verbose)
 			printf(_("suspend mode: no; leaving\n"));
 		ctl.dryrun = 1;	/* to skip disabling alarm at the end */
 		break;
-
 	case OFF_MODE:
 	{
 		char *arg[5];
@@ -606,10 +540,8 @@ int main(int argc, char **argv)
 		arg[i++] = "-P";
 		arg[i++] = "now";
 		arg[i]   = NULL;
-
 		if (!ctl.dryrun) {
 			execv(arg[0], arg);
-
 			warn(_("failed to execute %s"), _PATH_SHUTDOWN);
 			rc = EXIT_FAILURE;
 		}
@@ -621,7 +553,6 @@ int main(int argc, char **argv)
 
 		if (ctl.verbose)
 			printf(_("suspend mode: on; reading rtc\n"));
-
 		if (!ctl.dryrun) {
 			do {
 				t = read(fd, &data, sizeof data);
@@ -640,7 +571,6 @@ int main(int argc, char **argv)
 		if (ctl.verbose)
 			printf(_("suspend mode: disable; disabling alarm\n"));
 		break;
-
 	case SHOW_MODE:
 		if (ctl.verbose)
 			printf(_("suspend mode: show; printing alarm info\n"));
@@ -648,7 +578,6 @@ int main(int argc, char **argv)
 			rc = EXIT_FAILURE;
 		ctl.dryrun = 1;	/* don't really disable alarm in the end, just show */
 		break;
-
 	default:
 		if (ctl.verbose)
 			printf(_("suspend mode: %s; suspending system\n"), mode_str[suspend]);
@@ -670,7 +599,6 @@ int main(int argc, char **argv)
 			}
 		}
 	}
-
 	close(fd);
 	return rc;
 }
-- 
2.3.0


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

* [PATCH 12/13] rtcwake: make some command line options mutually exclusive
  2015-02-22 14:42 [PATCH 00/13] pull: rtcwake changes Sami Kerola
                   ` (10 preceding siblings ...)
  2015-02-22 14:42 ` [PATCH 11/13] rtcwake: improve coding style Sami Kerola
@ 2015-02-22 14:42 ` Sami Kerola
  2015-02-22 14:42 ` [PATCH 13/13] rtcwake: read accepted mode strings from /sys/power/state Sami Kerola
  2015-06-02 12:48 ` [PATCH 00/13] pull: rtcwake changes Karel Zak
  13 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2015-02-22 14:42 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

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

diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
index e6f2170..32a0ba0 100644
--- a/sys-utils/rtcwake.c
+++ b/sys-utils/rtcwake.c
@@ -36,6 +36,7 @@
 #include "c.h"
 #include "closestream.h"
 #include "nls.h"
+#include "optutils.h"
 #include "pathnames.h"
 #include "strutils.h"
 #include "timeutils.h"
@@ -411,6 +412,11 @@ int main(int argc, char **argv)
 		{"date",	required_argument,	0, OPT_DATE},
 		{0,		0,			0, 0  }
 	};
+	static const ul_excl_t excl[] = {
+		{ 'a', 'l', 'u' },
+		{ 's', 't', OPT_DATE },
+	};
+	int excl_st[ARRAY_SIZE(excl)] = UL_EXCL_STATUS_INIT;
 
 	setlocale(LC_ALL, "");
 	bindtextdomain(PACKAGE, LOCALEDIR);
@@ -418,6 +424,7 @@ int main(int argc, char **argv)
 	atexit(close_stdout);
 	while ((t = getopt_long(argc, argv, "A:ahd:lm:ns:t:uVv",
 					long_options, NULL)) != EOF) {
+		err_exclusive_options(t, long_options, excl, excl_st);
 		switch (t) {
 		case 'A':
 			/* for better compatibility with hwclock */
-- 
2.3.0


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

* [PATCH 13/13] rtcwake: read accepted mode strings from /sys/power/state
  2015-02-22 14:42 [PATCH 00/13] pull: rtcwake changes Sami Kerola
                   ` (11 preceding siblings ...)
  2015-02-22 14:42 ` [PATCH 12/13] rtcwake: make some command line options mutually exclusive Sami Kerola
@ 2015-02-22 14:42 ` Sami Kerola
  2015-06-02 12:48 ` [PATCH 00/13] pull: rtcwake changes Karel Zak
  13 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2015-02-22 14:42 UTC (permalink / raw)
  To: util-linux; +Cc: Sami Kerola

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

diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
index 32a0ba0..b838f13 100644
--- a/sys-utils/rtcwake.c
+++ b/sys-utils/rtcwake.c
@@ -51,29 +51,16 @@
 #define DEFAULT_DEVICE		"rtc0"
 
 enum rtc_modes {	/* manual page --mode option explains these. */
-	STANDBY_MODE = 0,
-	MEM_MODE,
-	FREEZE_MODE,
-	DISK_MODE,	/* end of Documentation/power/states.txt modes  */
+	ERROR_MODE = 0,	/* invalid user input */
+	SYSFS_MODE,
 	OFF_MODE,
 	NO_MODE,
 	ON_MODE,	/* smaller <- read the code */
 	DISABLE_MODE,	/* greater <- to understand */
 	SHOW_MODE,
-	ERROR_MODE	/* invalid user input */
 };
 
-/* what system power mode to use?  for now handle only standardized mode
- * names; eventually when systems define their own state names, parse
- * /sys/power/state
- *
- * "on" is used just to test the RTC alarm mechanism, bypassing all the
- * wakeup-from-sleep infrastructure.  */
-static const char *mode_str[] = {
-	[STANDBY_MODE] = "standby",
-	[MEM_MODE] = "mem",
-	[FREEZE_MODE] = "freeze",
-	[DISK_MODE] = "disk",
+static const char *rtcwake_mode_string[] = {
 	[OFF_MODE] = "off",
 	[NO_MODE] = "no",
 	[ON_MODE] = "on",
@@ -88,6 +75,7 @@ enum clock_modes {
 };
 
 struct rtcwake_control {
+	char *mode_str;			/* name of the requested mode */
 	char *adjfile;			/* adjtime file path */
 	enum clock_modes clock_mode;	/* hwclock timezone */
 	time_t sys_time;		/* system time */
@@ -238,23 +226,35 @@ static int setup_alarm(struct rtcwake_control *ctl, int fd, time_t *wakeup)
 	return 0;
 }
 
-static int is_suspend_available(const int suspend)
+static int is_suspend_available(const char *name)
 {
-	int rc;
-	char buf[32];
-	FILE *f = fopen(SYS_POWER_STATE_PATH, "r");
+	int rc = 0;
+	char *line = NULL, *s, *p;
+	size_t sz = 0;
+	FILE *fp = fopen(SYS_POWER_STATE_PATH, "r");
 
-	if (!f)
-		return -1;
-	if (fgets(buf, sizeof buf, f) == NULL)
-		rc = -1;
-	else
-		rc = strstr(buf, mode_str[suspend]) != NULL;
-	fclose(f);
+	if (!fp)
+		return 0;
+	if (getline(&line, &sz, fp) == -1) {
+		fclose(fp);
+		return 0;
+	}
+	s = line;
+	while (1) {
+		p = strsep(&s, " ");
+		if (p == NULL || *p == '\0')
+			break;
+		if (!strcmp(p, name)) {
+			rc = 1;
+			break;
+		}
+	}
+	free(line);
+	fclose(fp);
 	return rc;
 }
 
-static void suspend_system(struct rtcwake_control *ctl, int suspend)
+static void suspend_system(struct rtcwake_control *ctl)
 {
 	FILE	*f = fopen(SYS_POWER_STATE_PATH, "w");
 
@@ -263,7 +263,7 @@ static void suspend_system(struct rtcwake_control *ctl, int suspend)
 		return;
 	}
 	if (!ctl->dryrun) {
-		fprintf(f, "%s\n", mode_str[suspend]);
+		fprintf(f, "%s\n", ctl->mode_str);
 		fflush(f);
 	}
 	/* this executes after wake from suspend */
@@ -338,23 +338,17 @@ static int print_alarm(struct rtcwake_control *ctl, int fd)
 
 static int get_mode(const char *optarg)
 {
-	if (!strcmp(optarg, mode_str[STANDBY_MODE]))
-		return STANDBY_MODE;
-	if (!strcmp(optarg, mode_str[MEM_MODE]))
-		return MEM_MODE;
-	if (!strcmp(optarg, mode_str[DISK_MODE]))
-		return DISK_MODE;
-	if (!strcmp(optarg, mode_str[ON_MODE]))
+	if (is_suspend_available(optarg))
+		return SYSFS_MODE;
+	if (!strcmp(optarg, rtcwake_mode_string[ON_MODE]))
 		return ON_MODE;
-	if (!strcmp(optarg, mode_str[NO_MODE]))
+	if (!strcmp(optarg, rtcwake_mode_string[NO_MODE]))
 		return NO_MODE;
-	if (!strcmp(optarg, mode_str[OFF_MODE]))
+	if (!strcmp(optarg, rtcwake_mode_string[OFF_MODE]))
 		return OFF_MODE;
-	if (!strcmp(optarg, mode_str[FREEZE_MODE]))
-		return FREEZE_MODE;
-	if (!strcmp(optarg, mode_str[DISABLE_MODE]))
+	if (!strcmp(optarg, rtcwake_mode_string[DISABLE_MODE]))
 		return DISABLE_MODE;
-	if (!strcmp(optarg, mode_str[SHOW_MODE]))
+	if (!strcmp(optarg, rtcwake_mode_string[SHOW_MODE]))
 		return SHOW_MODE;
 	return ERROR_MODE;
 }
@@ -382,13 +376,14 @@ static int open_dev_rtc(const char *devname)
 int main(int argc, char **argv)
 {
 	struct rtcwake_control ctl = {
+		.mode_str = "suspend",		/* default mode */
 		.adjfile = _PATH_ADJTIME,
 		.clock_mode = CM_AUTO,
 		0
 	};
 	char *devname = DEFAULT_DEVICE;
 	unsigned seconds = 0;
-	int suspend = STANDBY_MODE;
+	int suspend = SYSFS_MODE;
 	int rc = EXIT_SUCCESS;
 	int t;
 	int fd;
@@ -442,6 +437,7 @@ int main(int argc, char **argv)
 		case 'm':
 			if ((suspend = get_mode(optarg)) == ERROR_MODE)
 				errx(EXIT_FAILURE, _("unrecognized suspend state '%s'"), optarg);
+			ctl.mode_str = optarg;
 			break;
 		case 'n':
 			ctl.dryrun = 1;
@@ -502,12 +498,8 @@ int main(int argc, char **argv)
 		printf(_("alarm %ld, sys_time %ld, rtc_time %ld, seconds %u\n"),
 				alarm, ctl.sys_time, ctl.rtc_time, seconds);
 	if (suspend < DISABLE_MODE) {
-		if (suspend < OFF_MODE && is_suspend_available(suspend) <= 0) {
-			errx(EXIT_FAILURE, _("suspend to \"%s\" unavailable"),
-			     mode_str[suspend]);
-		}
-		/* care about alarm setup only if the show or disable
-		 * modes are not set */
+		/* perform alarm setup when the show or disable modes are
+		 * not set */
 		if (alarm) {
 			if (alarm < ctl.sys_time)
 				errx(EXIT_FAILURE, _("time doesn't go backward to %s"),
@@ -523,7 +515,7 @@ int main(int argc, char **argv)
 				ctime(&alarm));
 		else
 			printf(_("%s: wakeup from \"%s\" using %s at %s"),
-				program_invocation_short_name, mode_str[suspend], devname,
+				program_invocation_short_name, ctl.mode_str, devname,
 				ctime(&alarm));
 		fflush(stdout);
 		xusleep(10 * 1000);
@@ -587,9 +579,9 @@ int main(int argc, char **argv)
 		break;
 	default:
 		if (ctl.verbose)
-			printf(_("suspend mode: %s; suspending system\n"), mode_str[suspend]);
+			printf(_("suspend mode: %s; suspending system\n"), ctl.mode_str);
 		sync();
-		suspend_system(&ctl, suspend);
+		suspend_system(&ctl);
 	}
 
 	if (!ctl.dryrun) {
-- 
2.3.0


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

* Re: [PATCH 06/13] rtcwake: add human readable --date timestamp format
  2015-02-22 14:42 ` [PATCH 06/13] rtcwake: add human readable --date timestamp format Sami Kerola
@ 2015-02-23 20:32   ` Benno Schulenberg
  0 siblings, 0 replies; 29+ messages in thread
From: Benno Schulenberg @ 2015-02-23 20:32 UTC (permalink / raw)
  To: Sami Kerola; +Cc: Util-Linux


On Sun, Feb 22, 2015, at 15:42, Sami Kerola wrote:
> +.BR \-\-date " \fItimestamp"

.BI \-\-date " timestamp"

> +Set the wakeup time to the value of the timestamp.  Format of the
> +timestmap can be any of the following:

s/the timestamp.  Format of the timestmap/
  \fItimestamp\fR.  The format of \fItimestamp\fR/

> +	fputs(_("     --date <timestamp>   date time of timestamp to wake\n"), out);

s:date time of timestamp to wake:date and/or time to wake:

> +				errx(EXIT_FAILURE, _("invalid time value \"%s\""), optarg);

s:time:date/time:

Benno

-- 
http://www.fastmail.com - IMAP accessible web-mail


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

* Re: [PATCH 04/13] rtcwake: remove RTC_ALM_READ and RTC_ALM_SET compatibility
  2015-02-22 14:42 ` [PATCH 04/13] rtcwake: remove RTC_ALM_READ and RTC_ALM_SET compatibility Sami Kerola
@ 2015-02-23 20:37   ` Benno Schulenberg
  2015-03-01 11:24     ` Sami Kerola
  0 siblings, 1 reply; 29+ messages in thread
From: Benno Schulenberg @ 2015-02-23 20:37 UTC (permalink / raw)
  To: Sami Kerola; +Cc: Util-Linux


On Sun, Feb 22, 2015, at 15:42, Sami Kerola wrote:
> The RTC_WKALM_RD and RTC_WKALM_SET have avaRilable since 2.6.17, and
> preferred way since 2007.  Keeping the availability to old interface is
> no longer needed.

s/have avaRilable/have been available/
s/the availability to old interface/the fallbacks to the old interface/

Benno

-- 
http://www.fastmail.com - The way an email service should be


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

* Re: [PATCH 11/13] rtcwake: improve coding style
  2015-02-22 14:42 ` [PATCH 11/13] rtcwake: improve coding style Sami Kerola
@ 2015-02-23 20:40   ` Benno Schulenberg
  2015-03-01 11:33     ` Sami Kerola
  0 siblings, 1 reply; 29+ messages in thread
From: Benno Schulenberg @ 2015-02-23 20:40 UTC (permalink / raw)
  To: Sami Kerola; +Cc: Util-Linux


On Sun, Feb 22, 2015, at 15:42, Sami Kerola wrote:
> -	/* this process works in RTC time, except when working
> -	 * with the system clock (which always uses UTC).
> -	 */
> +	/* this process works in RTC time, except when working with the
> +	 * system clock (which always uses UTC). */

Since this and the other comments ends with a period, it really
should start with a capital.  When changing the line anyway...

Benno

-- 
http://www.fastmail.com - Send your email first class


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

* Re: [PATCH 02/13] rtcwake: enumerate constant mode strings
  2015-02-22 14:42 ` [PATCH 02/13] rtcwake: enumerate constant mode strings Sami Kerola
@ 2015-02-24 12:34   ` Karel Zak
  2015-03-01 12:23     ` Sami Kerola
  0 siblings, 1 reply; 29+ messages in thread
From: Karel Zak @ 2015-02-24 12:34 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

On Sun, Feb 22, 2015 at 02:42:08PM +0000, Sami Kerola wrote:
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
>  sys-utils/rtcwake.c | 126 ++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 78 insertions(+), 48 deletions(-)
> 
> diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
> index 8b5f69c..372e620 100644
> --- a/sys-utils/rtcwake.c
> +++ b/sys-utils/rtcwake.c
> @@ -52,7 +52,37 @@
>  #define RTC_PATH		"/sys/class/rtc/%s/device/power/wakeup"
>  #define SYS_POWER_STATE_PATH	"/sys/power/state"
>  #define DEFAULT_DEVICE		"/dev/rtc0"
> -#define DEFAULT_MODE		"standby"
> +
> +enum rtc_modes {	/* manual page --mode option explains these. */
> +	STANDBY_MODE = 0,
> +	MEM_MODE,
> +	FREEZE_MODE,
> +	DISK_MODE,	/* end of Documentation/power/states.txt modes  */
> +	OFF_MODE,
> +	NO_MODE,
> +	ON_MODE,	/* smaller <- read the code */
> +	DISABLE_MODE,	/* greater <- to understand */
> +	SHOW_MODE,
> +	ERROR_MODE	/* invalid user input */
> +};

Please, don't use ERROR_MODE or so, just return -EINVAL from the
parser.

> +static int get_mode(const char *optarg)
> +{
> +	if (!strcmp(optarg, mode_str[STANDBY_MODE]))
> +		return STANDBY_MODE;
> +	if (!strcmp(optarg, mode_str[MEM_MODE]))
> +		return MEM_MODE;
> +	if (!strcmp(optarg, mode_str[DISK_MODE]))
> +		return DISK_MODE;
> +	if (!strcmp(optarg, mode_str[ON_MODE]))
> +		return ON_MODE;
> +	if (!strcmp(optarg, mode_str[NO_MODE]))
> +		return NO_MODE;
> +	if (!strcmp(optarg, mode_str[OFF_MODE]))
> +		return OFF_MODE;
> +	if (!strcmp(optarg, mode_str[FREEZE_MODE]))
> +		return FREEZE_MODE;
> +	if (!strcmp(optarg, mode_str[DISABLE_MODE]))
> +		return DISABLE_MODE;
> +	if (!strcmp(optarg, mode_str[SHOW_MODE]))
> +		return SHOW_MODE;

 {
    for (i = 0; i < ARRAY_SIZE(mode_str); i++)
         if (strcmp(optarg, mode_str[i])
             return i;

    return -EINVAL;
 }

> -	if (!alarm && !seconds && strcmp(suspend,"disable") &&
> -				  strcmp(suspend,"show")) {
> -
> +	if (!alarm && !seconds && suspend < DISABLE_MODE) {

 this ("<") is pretty fragile, use

         && (suspend != DISABLE_MODE || suspend != SHOW_MODE)

>  		warnx(_("must provide wake time (see -t and -s options)"));
>  		usage(stderr);

...

> -	if (strcmp(suspend, "show") && strcmp(suspend, "disable")) {
> -		if (strcmp(suspend, "no") && strcmp(suspend, "on") &&
> -		    strcmp(suspend, "off") && is_suspend_available(suspend) <= 0) {
> -			errx(EXIT_FAILURE, _("suspend to \"%s\" unavailable"), suspend);
> +	if (suspend < DISABLE_MODE) {
> +		if (suspend < OFF_MODE && is_suspend_available(suspend) <= 0) {
> +			errx(EXIT_FAILURE, _("suspend to \"%s\" unavailable"),
> +			     mode_str[suspend]);
  		}

 the same situation

    Karel

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

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

* Re: [PATCH 09/13] rtcwake: do not overwrite device name
  2015-02-22 14:42 ` [PATCH 09/13] rtcwake: do not overwrite device name Sami Kerola
@ 2015-02-24 13:00   ` Karel Zak
  2015-03-01 12:24     ` Sami Kerola
  0 siblings, 1 reply; 29+ messages in thread
From: Karel Zak @ 2015-02-24 13:00 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

On Sun, Feb 22, 2015 at 02:42:15PM +0000, Sami Kerola wrote:
> This improves error messaging, and allows freeing path to device when it
> is not needed.
> 
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
>  sys-utils/rtcwake.c | 44 +++++++++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 21 deletions(-)
> 
> diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
> index f951c9c..7f1e351 100644
> --- a/sys-utils/rtcwake.c
> +++ b/sys-utils/rtcwake.c
> @@ -49,7 +49,7 @@
>  
>  #define RTC_PATH		"/sys/class/rtc/%s/device/power/wakeup"

This is really bad name, what about SYS_WAKEUP_PATH_TEMPLATE ?

> +static int open_dev_rtc(const char *devname)
> +{
> +	int fd;
> +	char *devpath;
> +	size_t len = strlen(devname);
> +
> +	devpath = xmalloc(sizeof "/dev/" + len);
> +	memcpy(devpath, "/dev/", sizeof "/dev/");
> +	memcpy(devpath + strlen("/dev/"), devname, len + 1);

 if (startswith(devname, "/dev"))
    devpath = devname;
 else
    xasprintf(&devpath, "/dev/%s", devname);  

> +#ifdef O_CLOEXEC
> +	fd = open(devpath, O_RDONLY | O_CLOEXEC);
> +#else
> +	fd = open(devpath, O_RDONLY);
> +#endif

 we use O_CLOEXEC everywhere, #ifdef is unecessary

> +	if (fd < 0)
> +		err(EXIT_FAILURE, _("%s: unable to find device"), devpath);

 if (devname != devpath)
    free(devpath);

> +	free(devpath);
> +	return fd;
> +}

    Karel

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

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

* Re: [PATCH 10/13] bash-completion: add freeze mode to rtcwake
  2015-02-22 14:42 ` [PATCH 10/13] bash-completion: add freeze mode to rtcwake Sami Kerola
@ 2015-02-24 13:01   ` Karel Zak
  2015-03-01 12:26     ` Sami Kerola
  0 siblings, 1 reply; 29+ messages in thread
From: Karel Zak @ 2015-02-24 13:01 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

On Sun, Feb 22, 2015 at 02:42:16PM +0000, Sami Kerola wrote:
> Missed when commit ece44f19f423408f576f348fed2845c876d72c6e added support
> to freeze.
> 
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
>  bash-completion/rtcwake | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/bash-completion/rtcwake b/bash-completion/rtcwake
> index 51566a2..70ae743 100644
> --- a/bash-completion/rtcwake
> +++ b/bash-completion/rtcwake
> @@ -12,7 +12,7 @@ _rtcwake_module()
>  			return 0
>  			;;
>  		'-m'|'--mode')
> -			COMPREPLY=( $(compgen -W "standby mem disk off no on disable show" -- $cur) )
> +			COMPREPLY=( $(compgen -W "standby freeze mem disk off no on disable show" -- $cur) )

 would be possible to ask rtcwake(8) for all supported modes rather
 than hard code it to the bash-completion script?

    Karel


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

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

* Re: [PATCH 04/13] rtcwake: remove RTC_ALM_READ and RTC_ALM_SET compatibility
  2015-02-23 20:37   ` Benno Schulenberg
@ 2015-03-01 11:24     ` Sami Kerola
  0 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2015-03-01 11:24 UTC (permalink / raw)
  To: Benno Schulenberg; +Cc: Util-Linux

On Mon, 23 Feb 2015, Benno Schulenberg wrote:

> On Sun, Feb 22, 2015, at 15:42, Sami Kerola wrote:
> > The RTC_WKALM_RD and RTC_WKALM_SET have avaRilable since 2.6.17, and
> > preferred way since 2007.  Keeping the availability to old interface is
> > no longer needed.
> 
> s/have avaRilable/have been available/
> s/the availability to old interface/the fallbacks to the old interface/

Thanks Benno,

Here's updated commit message.

https://github.com/kerolasa/lelux-utiliteetit/commit/d14020f743a3d4dd5f1e4a7d4df4db5535ed0111

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

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

* Re: [PATCH 11/13] rtcwake: improve coding style
  2015-02-23 20:40   ` Benno Schulenberg
@ 2015-03-01 11:33     ` Sami Kerola
  2015-03-02 20:48       ` Benno Schulenberg
  0 siblings, 1 reply; 29+ messages in thread
From: Sami Kerola @ 2015-03-01 11:33 UTC (permalink / raw)
  To: Benno Schulenberg; +Cc: Util-Linux

On Mon, 23 Feb 2015, Benno Schulenberg wrote:

> On Sun, Feb 22, 2015, at 15:42, Sami Kerola wrote:
> > -	/* this process works in RTC time, except when working
> > -	 * with the system clock (which always uses UTC).
> > -	 */
> > +	/* this process works in RTC time, except when working with the
> > +	 * system clock (which always uses UTC). */
> 
> Since this and the other comments ends with a period, it really
> should start with a capital.  When changing the line anyway...

Oopps. I fixed the commits as part of other commit.

https://github.com/kerolasa/lelux-utiliteetit/commit/021378dd2baae75e42014b5e1cc595e8c1ab96bc

And missed the few time/date improvements you mentioned in mail:

Date: Mon, 23 Feb 2015 21:32:34 +0100
From: Benno Schulenberg <bensberg@justemail.net>
To: Sami Kerola <kerolasa@iki.fi>
Cc: Util-Linux <util-linux@vger.kernel.org>
Subject: Re: [PATCH 06/13] rtcwake: add human readable --date timestamp format

Perhaps string improvements can be done later with corrections to last(1) 
as well. If I remember right I copied the messages from last(1) pretty 
much as-is.

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

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

* Re: [PATCH 02/13] rtcwake: enumerate constant mode strings
  2015-02-24 12:34   ` Karel Zak
@ 2015-03-01 12:23     ` Sami Kerola
  0 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2015-03-01 12:23 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On Tue, 24 Feb 2015, Karel Zak wrote:

> On Sun, Feb 22, 2015 at 02:42:08PM +0000, Sami Kerola wrote:
> > Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> > ---
> >  sys-utils/rtcwake.c | 126 ++++++++++++++++++++++++++++++++--------------------
> >  1 file changed, 78 insertions(+), 48 deletions(-)
> > 
> > diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
> > index 8b5f69c..372e620 100644
> > --- a/sys-utils/rtcwake.c
> > +++ b/sys-utils/rtcwake.c
> > @@ -52,7 +52,37 @@
> >  #define RTC_PATH		"/sys/class/rtc/%s/device/power/wakeup"
> >  #define SYS_POWER_STATE_PATH	"/sys/power/state"
> >  #define DEFAULT_DEVICE		"/dev/rtc0"
> > -#define DEFAULT_MODE		"standby"
> > +
> > +enum rtc_modes {	/* manual page --mode option explains these. */
> > +	STANDBY_MODE = 0,
> > +	MEM_MODE,
> > +	FREEZE_MODE,
> > +	DISK_MODE,	/* end of Documentation/power/states.txt modes  */
> > +	OFF_MODE,
> > +	NO_MODE,
> > +	ON_MODE,	/* smaller <- read the code */
> > +	DISABLE_MODE,	/* greater <- to understand */
> > +	SHOW_MODE,
> > +	ERROR_MODE	/* invalid user input */
> > +};
> 
> Please, don't use ERROR_MODE or so, just return -EINVAL from the
> parser.
> 
> > +static int get_mode(const char *optarg)
> > +{
> > +	if (!strcmp(optarg, mode_str[STANDBY_MODE]))
> > +		return STANDBY_MODE;
> > +	if (!strcmp(optarg, mode_str[MEM_MODE]))
> > +		return MEM_MODE;
> > +	if (!strcmp(optarg, mode_str[DISK_MODE]))
> > +		return DISK_MODE;
> > +	if (!strcmp(optarg, mode_str[ON_MODE]))
> > +		return ON_MODE;
> > +	if (!strcmp(optarg, mode_str[NO_MODE]))
> > +		return NO_MODE;
> > +	if (!strcmp(optarg, mode_str[OFF_MODE]))
> > +		return OFF_MODE;
> > +	if (!strcmp(optarg, mode_str[FREEZE_MODE]))
> > +		return FREEZE_MODE;
> > +	if (!strcmp(optarg, mode_str[DISABLE_MODE]))
> > +		return DISABLE_MODE;
> > +	if (!strcmp(optarg, mode_str[SHOW_MODE]))
> > +		return SHOW_MODE;
> 
>  {
>     for (i = 0; i < ARRAY_SIZE(mode_str); i++)
>          if (strcmp(optarg, mode_str[i])
>              return i;
> 
>     return -EINVAL;
>  }
> 
> > -	if (!alarm && !seconds && strcmp(suspend,"disable") &&
> > -				  strcmp(suspend,"show")) {
> > -
> > +	if (!alarm && !seconds && suspend < DISABLE_MODE) {
> 
>  this ("<") is pretty fragile, use
> 
>          && (suspend != DISABLE_MODE || suspend != SHOW_MODE)
> 
> >  		warnx(_("must provide wake time (see -t and -s options)"));
> >  		usage(stderr);
> 
> ...
> 
> > -	if (strcmp(suspend, "show") && strcmp(suspend, "disable")) {
> > -		if (strcmp(suspend, "no") && strcmp(suspend, "on") &&
> > -		    strcmp(suspend, "off") && is_suspend_available(suspend) <= 0) {
> > -			errx(EXIT_FAILURE, _("suspend to \"%s\" unavailable"), suspend);
> > +	if (suspend < DISABLE_MODE) {
> > +		if (suspend < OFF_MODE && is_suspend_available(suspend) <= 0) {
> > +			errx(EXIT_FAILURE, _("suspend to \"%s\" unavailable"),
> > +			     mode_str[suspend]);
>   		}
> 
>  the same situation

Thanks Karel,

The get_mode() is simplified, and places where order of enum mattered 
(fragile) are made to be explicit what modes are/aren't allowed.

https://github.com/kerolasa/lelux-utiliteetit/commit/bfdcfc8177bc9b7c724ee0adc779083a01b13399

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

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

* Re: [PATCH 09/13] rtcwake: do not overwrite device name
  2015-02-24 13:00   ` Karel Zak
@ 2015-03-01 12:24     ` Sami Kerola
  0 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2015-03-01 12:24 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On Tue, 24 Feb 2015, Karel Zak wrote:

> On Sun, Feb 22, 2015 at 02:42:15PM +0000, Sami Kerola wrote:
> > This improves error messaging, and allows freeing path to device when it
> > is not needed.
> > 
> > Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> > ---
> >  sys-utils/rtcwake.c | 44 +++++++++++++++++++++++---------------------
> >  1 file changed, 23 insertions(+), 21 deletions(-)
> > 
> > diff --git a/sys-utils/rtcwake.c b/sys-utils/rtcwake.c
> > index f951c9c..7f1e351 100644
> > --- a/sys-utils/rtcwake.c
> > +++ b/sys-utils/rtcwake.c
> > @@ -49,7 +49,7 @@
> >  
> >  #define RTC_PATH		"/sys/class/rtc/%s/device/power/wakeup"
> 
> This is really bad name, what about SYS_WAKEUP_PATH_TEMPLATE ?
> 
> > +static int open_dev_rtc(const char *devname)
> > +{
> > +	int fd;
> > +	char *devpath;
> > +	size_t len = strlen(devname);
> > +
> > +	devpath = xmalloc(sizeof "/dev/" + len);
> > +	memcpy(devpath, "/dev/", sizeof "/dev/");
> > +	memcpy(devpath + strlen("/dev/"), devname, len + 1);
> 
>  if (startswith(devname, "/dev"))
>     devpath = devname;
>  else
>     xasprintf(&devpath, "/dev/%s", devname);  
> 
> > +#ifdef O_CLOEXEC
> > +	fd = open(devpath, O_RDONLY | O_CLOEXEC);
> > +#else
> > +	fd = open(devpath, O_RDONLY);
> > +#endif
> 
>  we use O_CLOEXEC everywhere, #ifdef is unecessary
> 
> > +	if (fd < 0)
> > +		err(EXIT_FAILURE, _("%s: unable to find device"), devpath);
> 
>  if (devname != devpath)
>     free(devpath);
> 
> > +	free(devpath);
> > +	return fd;
> > +}

Thanks Karel,

Updates are done according you proposals.

https://github.com/kerolasa/lelux-utiliteetit/commit/58cdd0c2880d616a7e3754fc629a5d28dd5256e7

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

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

* Re: [PATCH 10/13] bash-completion: add freeze mode to rtcwake
  2015-02-24 13:01   ` Karel Zak
@ 2015-03-01 12:26     ` Sami Kerola
  0 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2015-03-01 12:26 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On Tue, 24 Feb 2015, Karel Zak wrote:

> On Sun, Feb 22, 2015 at 02:42:16PM +0000, Sami Kerola wrote:
> > Missed when commit ece44f19f423408f576f348fed2845c876d72c6e added support
> > to freeze.
> > 
> > Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> > ---
> >  bash-completion/rtcwake | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/bash-completion/rtcwake b/bash-completion/rtcwake
> > index 51566a2..70ae743 100644
> > --- a/bash-completion/rtcwake
> > +++ b/bash-completion/rtcwake
> > @@ -12,7 +12,7 @@ _rtcwake_module()
> >  			return 0
> >  			;;
> >  		'-m'|'--mode')
> > -			COMPREPLY=( $(compgen -W "standby mem disk off no on disable show" -- $cur) )
> > +			COMPREPLY=( $(compgen -W "standby freeze mem disk off no on disable show" -- $cur) )
> 
>  would be possible to ask rtcwake(8) for all supported modes rather
>  than hard code it to the bash-completion script?

Thanks Karel,

Great idea. Change is updated to provide dynamic mode listing.

https://github.com/kerolasa/lelux-utiliteetit/commit/d827893773d9b565c42dd3300cdf4495891a08b2

Notice that the later change will update the listing to include only the 
modes from /sys/power/state.

https://github.com/kerolasa/lelux-utiliteetit/commit/9c147fa6ae5d4a8dcbb40e4bb8d6b82c408c52ed

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

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

* Re: [PATCH 11/13] rtcwake: improve coding style
  2015-03-01 11:33     ` Sami Kerola
@ 2015-03-02 20:48       ` Benno Schulenberg
  0 siblings, 0 replies; 29+ messages in thread
From: Benno Schulenberg @ 2015-03-02 20:48 UTC (permalink / raw)
  To: Sami Kerola; +Cc: Util-Linux


On Sun, Mar 1, 2015, at 12:33, Sami Kerola wrote:
> Oopps. I fixed the commits as part of other commit.
> 
> https://github.com/kerolasa/lelux-utiliteetit/commit/021378dd2baae75e42014b5e1cc595e8c1ab96bc

Hmm.  Can't you undo that?  And then do them right in the
"improve coding style" changeset?  And then improve the
"human-readable" changeset with the suggested changes?

If it's too much trouble, I'll supply a patch later.

Benno

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


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

* Re: [PATCH 00/13] pull: rtcwake changes
  2015-02-22 14:42 [PATCH 00/13] pull: rtcwake changes Sami Kerola
                   ` (12 preceding siblings ...)
  2015-02-22 14:42 ` [PATCH 13/13] rtcwake: read accepted mode strings from /sys/power/state Sami Kerola
@ 2015-06-02 12:48 ` Karel Zak
  2015-06-07 20:41   ` Sami Kerola
  13 siblings, 1 reply; 29+ messages in thread
From: Karel Zak @ 2015-06-02 12:48 UTC (permalink / raw)
  To: Sami Kerola; +Cc: util-linux

On Sun, Feb 22, 2015 at 02:42:06PM +0000, Sami Kerola wrote:
>  git://github.com/kerolasa/lelux-utiliteetit.git rtcwake

I have reviewed

   https://github.com/kerolasa/lelux-utiliteetit/commits/rtcwake2

and commented by github line notes.

    Karel

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

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

* Re: [PATCH 00/13] pull: rtcwake changes
  2015-06-02 12:48 ` [PATCH 00/13] pull: rtcwake changes Karel Zak
@ 2015-06-07 20:41   ` Sami Kerola
  2015-06-29 13:21     ` Karel Zak
  0 siblings, 1 reply; 29+ messages in thread
From: Sami Kerola @ 2015-06-07 20:41 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On 2 June 2015 at 13:48, Karel Zak <kzak@redhat.com> wrote:
> On Sun, Feb 22, 2015 at 02:42:06PM +0000, Sami Kerola wrote:
>>  git://github.com/kerolasa/lelux-utiliteetit.git rtcwake
>
> I have reviewed
>
>    https://github.com/kerolasa/lelux-utiliteetit/commits/rtcwake2
>
> and commented by github line notes.

Cheers. I left the rtcwake2 untouched, so that comments can be read
until final merge.

Some comments to comment.

> https://github.com/kerolasa/lelux-utiliteetit/commit/0358a2cec1f55c4a723cb55c4da7ea201d2acb68#diff-40ed35508632e222c29a58eba03a4271L374

Type is changed, but I left the while loop as is. It looks nicer to
me. Besides the get_mode() changes later in patch set again.

> https://github.com/kerolasa/lelux-utiliteetit/commit/0358a2cec1f55c4a723cb55c4da7ea201d2acb68#diff-40ed35508632e222c29a58eba03a4271L510

Well, that was dumb issue. Thank you pointing it out.

> https://github.com/kerolasa/lelux-utiliteetit/commit/cca324d3360535f0d990f7e696ebfd093b725c9c#diff-40ed35508632e222c29a58eba03a4271L317

skip_fline() is added to strutils.h and used the way as requested.

> https://github.com/kerolasa/lelux-utiliteetit/commit/d8c827a711e6e2d34ce01dc460d98c4a3759e04a#diff-40ed35508632e222c29a58eba03a4271L501

So do I.

> https://github.com/kerolasa/lelux-utiliteetit/commit/81f7e2a6fc5870ebfcb078e0973b0d9a57cae205#diff-40ed35508632e222c29a58eba03a4271R390

Done a bit differently with xstrdup(), so that I there's always an
allocation and free()'ing is simple.

> https://github.com/kerolasa/lelux-utiliteetit/commit/b9d49884a250e33c55f92e47d0e1b72e33598998#diff-40ed35508632e222c29a58eba03a4271L601

The strv.h is now used. I hope the code is more elegant, but to be
honest improvement might not be quite as nice as hoped. Notice that
the ctl->possible_modes is populated only when --list-modes and --mode
are in use, so I don't see any point running get_sys_power_states() at
early of execution.

BTW the new rtcwake3 has new commit. Just a simple fix.

https://github.com/kerolasa/lelux-utiliteetit/commit/a08b7936598fdc462c516331c4b91d331d7508ff
rtcwake: fix valgrind warning

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

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

* Re: [PATCH 00/13] pull: rtcwake changes
  2015-06-07 20:41   ` Sami Kerola
@ 2015-06-29 13:21     ` Karel Zak
  0 siblings, 0 replies; 29+ messages in thread
From: Karel Zak @ 2015-06-29 13:21 UTC (permalink / raw)
  To: kerolasa; +Cc: util-linux

On Sun, Jun 07, 2015 at 09:41:58PM +0100, Sami Kerola wrote:
> On 2 June 2015 at 13:48, Karel Zak <kzak@redhat.com> wrote:
> > On Sun, Feb 22, 2015 at 02:42:06PM +0000, Sami Kerola wrote:
> >>  git://github.com/kerolasa/lelux-utiliteetit.git rtcwake
> >
> > I have reviewed
> >
> >    https://github.com/kerolasa/lelux-utiliteetit/commits/rtcwake2
> >
> > and commented by github line notes.
> 
> Cheers. I left the rtcwake2 untouched, so that comments can be read
> until final merge.

 Merged (rtcwake3 branch) with some minor changes.

    Karel

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

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

end of thread, other threads:[~2015-06-29 13:21 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-22 14:42 [PATCH 00/13] pull: rtcwake changes Sami Kerola
2015-02-22 14:42 ` [PATCH 01/13] rtcwake: add rtcwake_control and remove global variables Sami Kerola
2015-02-22 14:42 ` [PATCH 02/13] rtcwake: enumerate constant mode strings Sami Kerola
2015-02-24 12:34   ` Karel Zak
2015-03-01 12:23     ` Sami Kerola
2015-02-22 14:42 ` [PATCH 03/13] rtcwake: replace long if else statement with switch case Sami Kerola
2015-02-22 14:42 ` [PATCH 04/13] rtcwake: remove RTC_ALM_READ and RTC_ALM_SET compatibility Sami Kerola
2015-02-23 20:37   ` Benno Schulenberg
2015-03-01 11:24     ` Sami Kerola
2015-02-22 14:42 ` [PATCH 05/13] rtcwake: improve read_clock_mode() Sami Kerola
2015-02-22 14:42 ` [PATCH 06/13] rtcwake: add human readable --date timestamp format Sami Kerola
2015-02-23 20:32   ` Benno Schulenberg
2015-02-22 14:42 ` [PATCH 07/13] rtcwake: fix preprocessor redefinition Sami Kerola
2015-02-22 14:42 ` [PATCH 08/13] rtcwake: clean up struct tm initializations Sami Kerola
2015-02-22 14:42 ` [PATCH 09/13] rtcwake: do not overwrite device name Sami Kerola
2015-02-24 13:00   ` Karel Zak
2015-03-01 12:24     ` Sami Kerola
2015-02-22 14:42 ` [PATCH 10/13] bash-completion: add freeze mode to rtcwake Sami Kerola
2015-02-24 13:01   ` Karel Zak
2015-03-01 12:26     ` Sami Kerola
2015-02-22 14:42 ` [PATCH 11/13] rtcwake: improve coding style Sami Kerola
2015-02-23 20:40   ` Benno Schulenberg
2015-03-01 11:33     ` Sami Kerola
2015-03-02 20:48       ` Benno Schulenberg
2015-02-22 14:42 ` [PATCH 12/13] rtcwake: make some command line options mutually exclusive Sami Kerola
2015-02-22 14:42 ` [PATCH 13/13] rtcwake: read accepted mode strings from /sys/power/state Sami Kerola
2015-06-02 12:48 ` [PATCH 00/13] pull: rtcwake changes Karel Zak
2015-06-07 20:41   ` Sami Kerola
2015-06-29 13:21     ` Karel Zak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox