When users give invalid arguments, tell them what they did wrong

Don't just dump a usage message and leave them to figure it out

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/test/x11perf/-/merge_requests/10>

1 files changed, 56 insertions, 30 deletions

diff --git a/x11perf.c b/x11perf.c
index 763d19d..eacea0d 100644
--- a/x11perf.c
+++ b/x11perf.c
@@ -326,6 +326,20 @@ typedef enum {
 } usage_contents;
 _X_NORETURN _X_COLD static void usage(usage_contents, int);
 
+_X_NORETURN _X_COLD static void
+missing_arg(const char *option)
+{
+    fprintf(stderr, "Error: missing argument to %s\n", option);
+    usage(USAGE_OPTIONS, EXIT_FAILURE);
+}
+
+_X_NORETURN _X_COLD static void
+invalid_arg( const char *arg, const char *option)
+{
+    fprintf(stderr, "Error: invalid argument to %s\n", arg, option);
+    usage(USAGE_OPTIONS, EXIT_FAILURE);
+}
+
 /*
  * Get_Display_Name (argc, argv) Look for -display, -d, or host:dpy (obsolete)
  * If found, remove it from command line.  Don't go past a lone -.
@@ -342,7 +356,8 @@ Get_Display_Name(int *pargc, /* MODIFIED */
 	char *arg = argv[i];
 
 	if (!strcmp (arg, "-display") || !strcmp (arg, "-d")) {
-	    if (++i >= argc) usage (USAGE_OPTIONS, EXIT_FAILURE);
+	    if (++i >= argc)
+		missing_arg(arg);
 
 	    displayname = argv[i];
 	    *pargc -= 2;
@@ -489,10 +504,10 @@ usage(usage_contents show, int exit_status)
 "    -all                      do all tests\n"
 "    -range <test1>[,<test2>]  like all, but do <test1> to <test2>\n"
 "    -labels                   generate test labels for use by fillblnk\n"
-"    -fg                       the foreground color to use\n"
-"    -bg                       the background color to use\n"
+"    -fg <color-or-pixel>      the foreground color to use\n"
+"    -bg <color-or-pixel>      the background color to use\n"
 "    -clips <default>          default number of clip windows per test\n"
-"    -ddbg                     the background color to use for DoubleDash\n"
+"    -ddbg <color-or-pixel>    the background color to use for DoubleDash\n"
 "    -rop <rop0 rop1 ...>      use the given rops to draw (default = GXcopy)\n"
 "    -pm <pm0 pm1 ...>         use the given planemasks to draw (default = ~0)\n"
 "    -depth <depth>            use a visual with <depth> planes per pixel\n"
@@ -937,7 +952,7 @@ main(int argc, char *argv[])
 	    char *cp2;
 
 	    if (argc <= ++i)
-		usage(USAGE_OPTIONS, EXIT_FAILURE);
+		missing_arg(argv[i-1]);
 	    cp1 = argv[i];
 	    if (*cp1 == '-')
 		cp1++;
@@ -958,13 +973,19 @@ main(int argc, char *argv[])
 		    } while (!(strcmp(cp2, (test[k].option + 1)) == 0 &&
 			       (test[k].versions & xparms.version)) &&
 			     test[++k].option != NULL);
-		    if (*cp2 != '-' && test[k].option == NULL)
+		    if (*cp2 != '-' && test[k].option == NULL) {
+			fprintf(stderr, "Error: unknown test %s listed for %s\n",
+				cp2, argv[i-1]);
 			usage(USAGE_OPTIONS, EXIT_FAILURE);
+		    }
 		    break;
 		}
 	    }
-	    if (test[j].option == NULL)
+	    if (test[j].option == NULL) {
+		fprintf(stderr, "Error: unknown test %s listed for %s\n",
+			argv[i], argv[i-1]);
 		usage(USAGE_OPTIONS, EXIT_FAILURE);
+	    }
 	    foundOne = True;
 	} else if (strcmp (argv[i], "-sync") == 0) {
 	    synchronous = True;
@@ -977,44 +998,44 @@ main(int argc, char *argv[])
 	} else if (strcmp (argv[i], "-repeat") == 0) {
 	    i++;
 	    if (argc <= i)
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+		missing_arg(argv[i-1]);
 	    repeat = atoi (argv[i]);
 	    if (repeat <= 0)
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+		invalid_arg(argv[i], argv[i-1]);
 	} else if (strcmp (argv[i], "-time") == 0) {
 	    i++;
 	    if (argc <= i)
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+		missing_arg(argv[i-1]);
 	    seconds = atoi (argv[i]);
 	    if (seconds <= 0)
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+		invalid_arg(argv[i], argv[i-1]);
         } else if (strcmp (argv[i], "-pause") == 0) {
             ++i;
 	    if (argc <= i)
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+		missing_arg(argv[i-1]);
 	    delay = atoi (argv[i]);
 	    if (delay < 0)
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+                invalid_arg(argv[i], argv[i-1]);
 	} else if (strcmp(argv[i], "-fg") == 0) {
 	    i++;
 	    if (argc <= i)
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+		missing_arg(argv[i-1]);
 	    foreground = argv[i];
         } else if (strcmp(argv[i], "-bg") == 0) {
 	    i++;
 	    if (argc <= i)
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+		missing_arg(argv[i-1]);
 	    background = argv[i];
 	    if(ddbackground == NULL)
 		ddbackground = argv[i];
 	} else if (strcmp(argv[i], "-clips") == 0 ) {
 	    i++;
 	    if (argc <= i)
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+		missing_arg(argv[i-1]);
 	    clips = atoi( argv[i] );
 	} else if (strcmp(argv[i], "-ddbg") == 0) {
 	    if (argc <= i)
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+		missing_arg(argv[i-1]);
 	    i++;
 	    ddbackground = argv[i];
 	} else if (strcmp(argv[i], "-rop") == 0) {
@@ -1039,21 +1060,21 @@ main(int argc, char *argv[])
 	} else if (strcmp(argv[i], "-reps") == 0) {
 	    i++;
 	    if (argc <= i)
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+		missing_arg(argv[i-1]);
 	    fixedReps = atoi (argv[i]);
 	    if (fixedReps <= 0)
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+		invalid_arg(argv[i], argv[i-1]);
         } else if (strcmp(argv[i], "-depth") == 0) {
 	    i++;
 	    if (argc <= i)
-                usage (USAGE_OPTIONS, EXIT_FAILURE);
+                missing_arg(argv[i-1]);
             depth = atoi(argv[i]);
             if (depth <= 0)
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+		invalid_arg(argv[i], argv[i-1]);
         } else if (strcmp(argv[i], "-vclass") == 0) {
 	    i++;
 	    if (argc <= i)
-                usage (USAGE_OPTIONS, EXIT_FAILURE);
+                missing_arg(argv[i-1]);
 	    for (j = StaticGray; j <= DirectColor; j++) {
 		if (strcmp(argv[i], visualClassNames[j]) == 0) {
 		    vclass = j;
@@ -1061,7 +1082,7 @@ main(int argc, char *argv[])
 		}
 	    }
             if (vclass < 0)
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+                invalid_arg(argv[i], argv[i-1]);
 	} else if (strcmp(argv[i], "-subs") == 0) {
 	    skip = GetNumbers (i+1, argc, argv, subWindows, &numSubWindows);
 	    i += skip;
@@ -1070,12 +1091,13 @@ main(int argc, char *argv[])
 	} else if (strcmp(argv[i], "-bs") == 0) {
 	    i++;
 	    if (argc <= i)
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+		missing_arg(argv[i-1]);
 	    if (strcmp(argv[i], "WhenMapped") == 0) {
 	      xparms.backing_store = WhenMapped;
 	    } else if (strcmp(argv[i], "Always") == 0) {
 	      xparms.backing_store = Always;
-	    } else usage (USAGE_OPTIONS, EXIT_FAILURE);
+	    } else
+                invalid_arg(argv[i], argv[i-1]);
 	} else if ((strcmp(argv[i], "-version") == 0) ||
 		   (strcmp(argv[i], "--version") == 0)) {
 	    puts(PACKAGE_STRING);
@@ -1094,8 +1116,7 @@ main(int argc, char *argv[])
 		usage (USAGE_ALL, EXIT_SUCCESS);
 	    }
 	    else {
-		fprintf(stderr, "unknown --help argument: %s\n", argv[i]);
-		usage (USAGE_OPTIONS, EXIT_FAILURE);
+		invalid_arg(argv[i], argv[i-1]);
 	    }
 	} else {
 	    int len,found;
@@ -1122,8 +1143,11 @@ main(int argc, char *argv[])
 		    doit[j] = found = True;
 		}
 	    }
-	    if(!found)
+	    if (!found) {
+		fprintf(stderr,
+			"Error: unrecognized option %s\n", argv[i]);
 		usage (USAGE_OPTIONS, EXIT_FAILURE);
+	    }
 	LegalOption:
 		foundOne = True;
 	}
@@ -1198,8 +1222,10 @@ main(int argc, char *argv[])
 	exit(0);
     }
 
-    if (!foundOne)
+    if (!foundOne) {
+	fprintf(stderr, "Error: no argument found for which test(s) to run\n");
 	usage (USAGE_OPTIONS, EXIT_FAILURE);
+    }
     xparms.d = Open_Display (displayName);
     screen = DefaultScreen(xparms.d);
 
@@ -1445,7 +1471,7 @@ GetWords (int argi, int argc, char **argv, char **wordsp, int *nump)
     int	    count;
 
     if (argc <= argi)
-	usage(USAGE_OPTIONS, EXIT_FAILURE);
+	missing_arg(argv[argi-1]);
     count = 0;
     while (argv[argi] && *(argv[argi]) != '-') {
 	*wordsp++ = argv[argi];