[CRIU] [PATCH 2/4] criu: unify/improve bad argument messages

Kir Kolyshkin kir at openvz.org
Wed Jan 8 19:33:03 PST 2014


* Introduce a generic way to report that option argument is invalid
* Switch to using it from existing places
  (options --veth-pair, --port, -n)
* Check for invalid argument of -p and -t and report it.

Notes:

1) In order to correctly print long option name in case it was used
   instead of a short one, I had to move "struct option long_opts"
   to main() context, this is why the patch is so long.

2) pr_msg() (rather than pr_err()) is used to print errors, otherwise
   it is prefixed with that (crtools.c:123) prefix which makes it
   look weird.

3) Usage is not shown in case of error, otherwise an error message
   is lost in output.

Signed-off-by: Kir Kolyshkin <kir at openvz.org>
---
 crtools.c | 104 ++++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 57 insertions(+), 47 deletions(-)

diff --git a/crtools.c b/crtools.c
index e65d470..6a16f89 100644
--- a/crtools.c
+++ b/crtools.c
@@ -83,6 +83,45 @@ int main(int argc, char *argv[])
 	int log_level = 0;
 	char *imgs_dir = ".";
 	char *work_dir = NULL;
+	static const char short_opts[] = "dsRf:F:t:p:hcD:o:n:v::xVr:jlW:L:";
+	static struct option long_opts[] = {
+		{ "tree", required_argument, 0, 't' },
+		{ "pid", required_argument, 0, 'p' },
+		{ "leave-stopped", no_argument, 0, 's' },
+		{ "leave-running", no_argument, 0, 'R' },
+		{ "restore-detached", no_argument, 0, 'd' },
+		{ "daemon", no_argument, 0, 'd' },
+		{ "contents", no_argument, 0, 'c' },
+		{ "file", required_argument, 0, 'f' },
+		{ "fields", required_argument, 0, 'F' },
+		{ "images-dir", required_argument, 0, 'D' },
+		{ "work-dir", required_argument, 0, 'W' },
+		{ "log-file", required_argument, 0, 'o' },
+		{ "namespaces", required_argument, 0, 'n' },
+		{ "root", required_argument, 0, 'r' },
+		{ USK_EXT_PARAM, no_argument, 0, 'x' },
+		{ "help", no_argument, 0, 'h' },
+		{ SK_EST_PARAM, no_argument, 0, 42 },
+		{ "close", required_argument, 0, 43 },
+		{ "log-pid", no_argument, 0, 44},
+		{ "version", no_argument, 0, 'V'},
+		{ "evasive-devices", no_argument, 0, 45},
+		{ "pidfile", required_argument, 0, 46},
+		{ "veth-pair", required_argument, 0, 47},
+		{ "action-script", required_argument, 0, 49},
+		{ LREMAP_PARAM, no_argument, 0, 41},
+		{ OPT_SHELL_JOB, no_argument, 0, 'j'},
+		{ OPT_FILE_LOCKS, no_argument, 0, 'l'},
+		{ "page-server", no_argument, 0, 50},
+		{ "address", required_argument, 0, 51},
+		{ "port", required_argument, 0, 52},
+		{ "prev-images-dir", required_argument, 0, 53},
+		{ "ms", no_argument, 0, 54},
+		{ "track-mem", no_argument, 0, 55},
+		{ "auto-dedup", no_argument, 0, 56},
+		{ "libdir", required_argument, 0, 'L'},
+		{ },
+	};
 
 	BUILD_BUG_ON(PAGE_SIZE != PAGE_IMAGE_SIZE);
 
@@ -98,46 +137,7 @@ int main(int argc, char *argv[])
 		return 1;
 
 	while (1) {
-		static const char short_opts[] = "dsRf:F:t:p:hcD:o:n:v::xVr:jlW:L:";
-		static struct option long_opts[] = {
-			{ "tree", required_argument, 0, 't' },
-			{ "pid", required_argument, 0, 'p' },
-			{ "leave-stopped", no_argument, 0, 's' },
-			{ "leave-running", no_argument, 0, 'R' },
-			{ "restore-detached", no_argument, 0, 'd' },
-			{ "daemon", no_argument, 0, 'd' },
-			{ "contents", no_argument, 0, 'c' },
-			{ "file", required_argument, 0, 'f' },
-			{ "fields", required_argument, 0, 'F' },
-			{ "images-dir", required_argument, 0, 'D' },
-			{ "work-dir", required_argument, 0, 'W' },
-			{ "log-file", required_argument, 0, 'o' },
-			{ "namespaces", required_argument, 0, 'n' },
-			{ "root", required_argument, 0, 'r' },
-			{ USK_EXT_PARAM, no_argument, 0, 'x' },
-			{ "help", no_argument, 0, 'h' },
-			{ SK_EST_PARAM, no_argument, 0, 42 },
-			{ "close", required_argument, 0, 43 },
-			{ "log-pid", no_argument, 0, 44},
-			{ "version", no_argument, 0, 'V'},
-			{ "evasive-devices", no_argument, 0, 45},
-			{ "pidfile", required_argument, 0, 46},
-			{ "veth-pair", required_argument, 0, 47},
-			{ "action-script", required_argument, 0, 49},
-			{ LREMAP_PARAM, no_argument, 0, 41},
-			{ OPT_SHELL_JOB, no_argument, 0, 'j'},
-			{ OPT_FILE_LOCKS, no_argument, 0, 'l'},
-			{ "page-server", no_argument, 0, 50},
-			{ "address", required_argument, 0, 51},
-			{ "port", required_argument, 0, 52},
-			{ "prev-images-dir", required_argument, 0, 53},
-			{ "ms", no_argument, 0, 54},
-			{ "track-mem", no_argument, 0, 55},
-			{ "auto-dedup", no_argument, 0, 56},
-			{ "libdir", required_argument, 0, 'L'},
-			{ },
-		};
-
+		idx = -1;
 		opt = getopt_long(argc, argv, short_opts, long_opts, &idx);
 		if (opt == -1)
 			break;
@@ -154,9 +154,13 @@ int main(int argc, char *argv[])
 			break;
 		case 'p':
 			pid = atoi(optarg);
+			if (pid <= 0)
+				goto bad_arg;
 			break;
 		case 't':
 			tree_id = atoi(optarg);
+			if (tree_id <= 0)
+				goto bad_arg;
 			break;
 		case 'c':
 			opts.show_pages_content	= true;
@@ -184,7 +188,7 @@ int main(int argc, char *argv[])
 			break;
 		case 'n':
 			if (parse_ns_string(optarg))
-				return 1;
+				goto bad_arg;
 			break;
 		case 'v':
 			if (optarg) {
@@ -231,8 +235,7 @@ int main(int argc, char *argv[])
 				n->outside = strchr(optarg, '=');
 				if (n->outside == NULL) {
 					xfree(n);
-					pr_err("Invalid argument for --veth-pair\n");
-					goto usage;
+					goto bad_arg;
 				}
 
 				*n->outside++ = '\0';
@@ -260,10 +263,8 @@ int main(int argc, char *argv[])
 			break;
 		case 52:
 			opts.ps_port = htons(atoi(optarg));
-			if (!opts.ps_port) {
-				pr_err("Bad port\n");
-				return 1;
-			}
+			if (!opts.ps_port)
+				goto bad_arg;
 			break;
 		case 'j':
 			opts.shell_job = true;
@@ -469,4 +470,13 @@ usage:
 opt_pid_missing:
 	pr_msg("No pid specified (-t option missing)\n");
 	return 1;
+
+bad_arg:
+	if (idx < 0) /* short option */
+		pr_msg("Error: invalid argument for -%c: %s\n",
+				opt, optarg);
+	else /* long option */
+		pr_msg("Error: invalid argument for --%s: %s\n",
+				long_opts[idx].name, optarg);
+	return 1;
 }
-- 
1.8.1.4



More information about the CRIU mailing list