From c23a867485e3dfe472d849b2a9caaf80dd732437 Mon Sep 17 00:00:00 2001
From: Tobias Waldekranz <tobias@waldekranz.com>
Date: Mon, 6 Feb 2023 15:48:27 +0100
Subject: [PATCH] initctl: Follow priciple of least surprise for "cond get"

The accepted standard in Unix is to report successful executions with
exitcode 0. Therefore, map a "initctl cond get" of a condition to the
following exitcodes:

- On: 0
- Off: 1
- Flux: 255

Fixes: c3c662fe6446 ("initctl: ensure 'cond get' support flux state")
Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 man/initctl.8 |  2 +-
 src/initctl.c | 34 +++++++++++++++++++---------------
 2 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/man/initctl.8 b/man/initctl.8
index 07ef8dfc..29909c92 100644
--- a/man/initctl.8
+++ b/man/initctl.8
@@ -145,7 +145,7 @@ or
 then any condition can be read.
 .Pp
 The command is geared for scripting, check the exit code to get the
-status of the condition: 0 - off, 1 - on, 255 - flux.  For a more
+status of the condition: 0 - on, 1 - off, 255 - flux.  For a more
 verbose output, use the
 .Fl v
 option.
diff --git a/src/initctl.c b/src/initctl.c
index 01aa720e..d2f5f6a0 100644
--- a/src/initctl.c
+++ b/src/initctl.c
@@ -409,24 +409,24 @@ static int do_cond_dump(char *arg)
 
 typedef enum { COND_CLR, COND_SET, COND_GET } condop_t;
 
-static int cond_read(char *path)
+static cond_state_t cond_read(char *path)
 {
 	int now, gen;
 
 	if (fngetint(path, &gen) == -1)
-		return 0;
+		return COND_OFF;
 
 	/*
 	 * if we cannot read the reconf generation, then either sth is
 	 * very wrong, or we are called very early/late boot/shutdown.
 	 */
 	if (fngetint(_PATH_RECONF, &now) == -1)
-		return 255;	/* classify as flux */
+		return COND_FLUX;	/* classify as flux */
 
 	if (now != gen)
-		return 255;
+		return COND_FLUX;
 
-	return 1;
+	return COND_ON;
 }
 
 /*
@@ -435,9 +435,9 @@ static int cond_read(char *path)
  */
 static int do_cond_act(char *args, condop_t op)
 {
+	cond_state_t cstate;
 	char path[256];
 	char *arg;
-	int rc;
 
 	if (!args || !args[0])
 		ERRX(2, "Invalid condition (empty)");
@@ -465,16 +465,20 @@ static int do_cond_act(char *args, condop_t op)
 
 		switch (op) {
 		case COND_GET:
-			rc = cond_read(path);
-			if (verbose) {
-				if (rc == 255)
-					puts("flux");
-				else if (rc == 0)
-					puts("off");
-				else
-					puts("on");
+			cstate = cond_read(path);
+			if (verbose)
+				puts(condstr(cstate));
+
+			switch (cstate) {
+			case COND_ON:
+				return 0;
+			case COND_OFF:
+				return 1;
+			case COND_FLUX:
+			default:
+				break;
 			}
-			return rc;
+			return 255;
 
 		case COND_SET:
 			if (symlink(_PATH_RECONF, path) && errno != EEXIST)