[PATCH 2/2] strutils: move an error message to where it is used, and improve it

Benno Schulenberg <[email protected]>
Newsgroups org.kernel.vger.util-linux
Message-ID <[email protected]>
Calling gettext() is somewhat costly: it has to find the given message
among the more than five thousand messages in util-linux's repertoire.
So, call gettext() only when the message actually gets printed.

Besides, allowing to customize the error message for parse_switch() was
a nice gesture, but it's unneeded: a fixed error message is good enough.

Also, "argument error" was rather vague, as it doesn't say _what_ the
error is.  Better say "unsupported argument".

Signed-off-by: Benno Schulenberg <[email protected]>
---
 include/strutils.h   |  2 +-
 lib/strutils.c       |  8 ++++----
 sys-utils/eject.c    |  6 ++----
 sys-utils/irqtop.c   |  3 +--
 sys-utils/losetup.c  |  2 +-
 sys-utils/tunelp.c   |  8 ++++----
 term-utils/setterm.c | 36 ++++++++++++------------------------
 7 files changed, 25 insertions(+), 40 deletions(-)

diff --git a/include/strutils.h b/include/strutils.h
index 63cd1c1c6..f8eb5f7c4 100644
--- a/include/strutils.h
+++ b/include/strutils.h
@@ -68,7 +68,7 @@ extern int isxdigit_strend(const char *str, const char **end);
 #define isxdigit_string(_s)	isxdigit_strend(_s, NULL)
 
 
-extern int parse_switch(const char *arg, const char *errmesg, ...);
+extern int parse_switch(const char *arg, ...);
 
 #ifndef HAVE_MEMPCPY
 extern void *mempcpy(void *restrict dest, const void *restrict src, size_t n);
diff --git a/lib/strutils.c b/lib/strutils.c
index 0cf0da96b..00934d511 100644
--- a/lib/strutils.c
+++ b/lib/strutils.c
@@ -247,14 +247,14 @@ int isxdigit_strend(const char *str, const char **end)
 }
 
 /*
- *  parse_switch(argv[i], "on", "off",  "yes", "no",  NULL);
+ *  For example: parse_switch(argv[i], "on", "off",  "yes", "no",  NULL);
  */
-int parse_switch(const char *arg, const char *errmesg, ...)
+int parse_switch(const char *arg, ...)
 {
 	const char *a, *b;
 	va_list ap;
 
-	va_start(ap, errmesg);
+	va_start(ap, *arg);
 	do {
 		a = va_arg(ap, char *);
 		if (!a)
@@ -275,7 +275,7 @@ int parse_switch(const char *arg, const char *errmesg, ...)
 	} while (1);
 	va_end(ap);
 
-	errx(STRTOXX_EXIT_CODE, "%s: '%s'", errmesg, arg);
+	errx(STRTOXX_EXIT_CODE, _("unsupported argument: %s"), arg);
 }
 
 #ifndef HAVE_MEMPCPY
diff --git a/sys-utils/eject.c b/sys-utils/eject.c
index 310d6c3df..d01cde719 100644
--- a/sys-utils/eject.c
+++ b/sys-utils/eject.c
@@ -207,8 +207,7 @@ static void parse_args(struct eject_control *ctl, int argc, char **argv)
 		switch (c) {
 		case 'a':
 			ctl->a_option = 1;
-			ctl->a_arg = parse_switch(optarg, _("argument error"),
-						"on", "off",  "1", "0",  NULL);
+			ctl->a_arg = parse_switch(optarg, "on", "off",  "1", "0",  NULL);
 			break;
 		case 'c':
 			ctl->c_option = 1;
@@ -229,8 +228,7 @@ static void parse_args(struct eject_control *ctl, int argc, char **argv)
 			break;
 		case 'i':
 			ctl->i_option = 1;
-			ctl->i_arg = parse_switch(optarg, _("argument error"),
-						"on", "off",  "1", "0",  NULL);
+			ctl->i_arg = parse_switch(optarg, "on", "off",  "1", "0",  NULL);
 			break;
 		case 'm':
 			ctl->m_option = 1;
diff --git a/sys-utils/irqtop.c b/sys-utils/irqtop.c
index 51e04ee85..7275d0510 100644
--- a/sys-utils/irqtop.c
+++ b/sys-utils/irqtop.c
@@ -366,9 +366,8 @@ static void parse_args(	struct irqtop_ctl *ctl,
 				ctl->cpustat_mode = IRQTOP_CPUSTAT_AUTO;
 			else
 				ctl->cpustat_mode = IRQTOP_CPUSTAT_DISABLE - parse_switch(optarg,
-							_("unsupported argument"), "yes", "no",
 							"always", "never", "enable", "disable",
-							"on", "off", "1", "0", NULL);
+							"on", "off", "yes", "no", "1", "0", NULL);
 			break;
 		case 'C':
 			{
diff --git a/sys-utils/losetup.c b/sys-utils/losetup.c
index eafab72f3..2840586a8 100644
--- a/sys-utils/losetup.c
+++ b/sys-utils/losetup.c
@@ -818,7 +818,7 @@ int main(int argc, char **argv)
 		case OPT_DIO:
 			use_dio = set_dio = 1;
 			if (optarg)
-				use_dio = parse_switch(optarg, _("argument error"), "on", "off", NULL);
+				use_dio = parse_switch(optarg, "on", "off", NULL);
 			if (use_dio)
 				lo_flags |= LO_FLAGS_DIRECT_IO;
 			break;
diff --git a/sys-utils/tunelp.c b/sys-utils/tunelp.c
index 95a21b39e..51442d595 100644
--- a/sys-utils/tunelp.c
+++ b/sys-utils/tunelp.c
@@ -189,24 +189,24 @@ int main(int argc, char **argv)
 			break;
 		case 'a':
 			cmds->op = LPABORT;
-			cmds->val = parse_switch(optarg, _("argument error"), "on", "off", NULL);
+			cmds->val = parse_switch(optarg, "on", "off", NULL);
 			cmds->next = xmalloc(sizeof(struct command));
 			cmds = cmds->next;
 			cmds->next = NULL;
 			break;
 		case 'q':
-			show_irq = parse_switch(optarg, _("argument error"), "on", "off", NULL);
+			show_irq = parse_switch(optarg, "on", "off", NULL);
 			break;
 		case 'o':
 			cmds->op = LPABORTOPEN;
-			cmds->val = parse_switch(optarg, _("argument error"), "on", "off", NULL);
+			cmds->val = parse_switch(optarg, "on", "off", NULL);
 			cmds->next = xmalloc(sizeof(struct command));
 			cmds = cmds->next;
 			cmds->next = NULL;
 			break;
 		case 'C':
 			cmds->op = LPCAREFUL;
-			cmds->val = parse_switch(optarg, _("argument error"), "on", "off", NULL);
+			cmds->val = parse_switch(optarg, "on", "off", NULL);
 			cmds->next = xmalloc(sizeof(struct command));
 			cmds = cmds->next;
 			cmds->next = NULL;
diff --git a/term-utils/setterm.c b/term-utils/setterm.c
index 801095c87..886ff78fc 100644
--- a/term-utils/setterm.c
+++ b/term-utils/setterm.c
@@ -559,23 +559,19 @@ static void parse_option(struct setterm_control *ctl, int ac, char **av)
 			break;
 		case OPT_CURSOR:
 			ctl->opt_cursor = set_opt_flag(ctl->opt_cursor);
-			ctl->opt_cu_on = parse_switch(optarg, _("argument error"),
-						"on", "off", NULL);
+			ctl->opt_cu_on = parse_switch(optarg,"on", "off", NULL);
 			break;
 		case OPT_REPEAT:
 			ctl->opt_repeat = set_opt_flag(ctl->opt_repeat);
-			ctl->opt_rep_on = parse_switch(optarg, _("argument error"),
-						"on", "off", NULL);
+			ctl->opt_rep_on = parse_switch(optarg, "on", "off", NULL);
 			break;
 		case OPT_APPCURSORKEYS:
 			ctl->opt_appcursorkeys = set_opt_flag(ctl->opt_appcursorkeys);
-			ctl->opt_appck_on = parse_switch(optarg, _("argument error"),
-						"on", "off", NULL);
+			ctl->opt_appck_on = parse_switch(optarg, "on", "off", NULL);
 			break;
 		case OPT_LINEWRAP:
 			ctl->opt_linewrap = set_opt_flag(ctl->opt_linewrap);
-			ctl->opt_li_on = parse_switch(optarg, _("argument error"),
-						"on", "off", NULL);
+			ctl->opt_li_on = parse_switch(optarg, "on", "off", NULL);
 			break;
 		case OPT_DEFAULT:
 			ctl->opt_default = set_opt_flag(ctl->opt_default);
@@ -598,33 +594,27 @@ static void parse_option(struct setterm_control *ctl, int ac, char **av)
 			break;
 		case OPT_INVERSESCREEN:
 			ctl->opt_inversescreen = set_opt_flag(ctl->opt_inversescreen);
-			ctl->opt_invsc_on = parse_switch(optarg, _("argument error"),
-						"on", "off", NULL);
+			ctl->opt_invsc_on = parse_switch(optarg, "on", "off", NULL);
 			break;
 		case OPT_BOLD:
 			ctl->opt_bold = set_opt_flag(ctl->opt_bold);
-			ctl->opt_bo_on = parse_switch(optarg, _("argument error"),
-						"on", "off", NULL);
+			ctl->opt_bo_on = parse_switch(optarg, "on", "off", NULL);
 			break;
 		case OPT_HALF_BRIGHT:
 			ctl->opt_halfbright = set_opt_flag(ctl->opt_halfbright);
-			ctl->opt_hb_on = parse_switch(optarg, _("argument error"),
-						"on", "off", NULL);
+			ctl->opt_hb_on = parse_switch(optarg, "on", "off", NULL);
 			break;
 		case OPT_BLINK:
 			ctl->opt_blink = set_opt_flag(ctl->opt_blink);
-			ctl->opt_bl_on = parse_switch(optarg, _("argument error"),
-						"on", "off", NULL);
+			ctl->opt_bl_on = parse_switch(optarg, "on", "off", NULL);
 			break;
 		case OPT_REVERSE:
 			ctl->opt_reverse = set_opt_flag(ctl->opt_reverse);
-			ctl->opt_re_on = parse_switch(optarg, _("argument error"),
-						"on", "off", NULL);
+			ctl->opt_re_on = parse_switch(optarg, "on", "off", NULL);
 			break;
 		case OPT_UNDERLINE:
 			ctl->opt_underline = set_opt_flag(ctl->opt_underline);
-			ctl->opt_un_on = parse_switch(optarg, _("argument error"),
-						"on", "off", NULL);
+			ctl->opt_un_on = parse_switch(optarg, "on", "off", NULL);
 			break;
 		case OPT_STORE:
 			ctl->opt_store = set_opt_flag(ctl->opt_store);
@@ -632,8 +622,7 @@ static void parse_option(struct setterm_control *ctl, int ac, char **av)
 		case OPT_CLEAR:
 			ctl->opt_clear = set_opt_flag(ctl->opt_clear);
 			if (optarg)
-				ctl->opt_cl_all = parse_switch(optarg, _("argument error"),
-						"all", "rest", NULL);
+				ctl->opt_cl_all = parse_switch(optarg, "all", "rest", NULL);
 			else
 				ctl->opt_cl_all = 1;
 			break;
@@ -667,8 +656,7 @@ static void parse_option(struct setterm_control *ctl, int ac, char **av)
 			break;
 		case OPT_MSG:
 			ctl->opt_msg = set_opt_flag(ctl->opt_msg);
-			ctl->opt_msg_on = parse_switch(optarg, _("argument error"),
-						"on", "off", NULL);
+			ctl->opt_msg_on = parse_switch(optarg, "on", "off", NULL);
 			break;
 		case OPT_MSGLEVEL:
 			ctl->opt_msglevel = set_opt_flag(ctl->opt_msglevel);
-- 
2.48.1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.