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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <bensberg@xxxxxxxxxx>
---
 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





[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux