lib/igt_core: Rework extra options conflicts handling

This started simple, as a fixup for a warning:
  In file included from ../lib/drmtest.h:39,
                   from ../lib/igt_core.c:60:
  ../lib/igt_core.c: In function ‘common_init’:
  ../lib/igt_core.h:891:24: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
    891 | #define igt_warn(f...) igt_log(IGT_LOG_DOMAIN, IGT_LOG_WARN, f)
        |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ../lib/igt_core.c:704:4: note: in expansion of macro ‘igt_warn’
    704 |    igt_warn("Conflicting long and short option values between --%s and -%s\n",
        |    ^~~~~~~~
  ../lib/igt_core.c:704:73: note: format string is defined here
    704 |    igt_warn("Conflicting long and short option values between --%s and -%s\n",
        |                                                                         ^~

But it ended up doing the following things:
  1. Promote all igt_warns to _critical and assert afterwards.
  2. Use for loop instead of a while-doing-for's-job.
  3. Streamline calculation of the option list sizes.
  4. Add checks for long option names.
  5. Log about "'val' representation" instead of confusing "value".
  6. Log correct things so we won't %s on a NULL.
  7. Write tests to confirm that it works.

Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Petri Latvala <petri.latvala@intel.com>

2 files changed, 100 insertions, 0 deletions

diff --git a/lib/tests/igt_conflicting_args.c b/lib/tests/igt_conflicting_args.c
new file mode 100644
index 00000000..c357b6c5
--- /dev/null
+++ b/lib/tests/igt_conflicting_args.c
@@ -0,0 +1,99 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/*
+ * DESCRIPTION: Make sure that IGT framework complains when test try to define
+ * conflicting options.
+ */
+
+#include <signal.h>
+#include <sys/wait.h>
+#include <errno.h>
+
+#include "drmtest.h"
+#include "igt_core.h"
+#include "igt_tests_common.h"
+
+static struct option long_options[2];
+static const char *short_options;
+
+static int opt_handler(int option, int option_index, void *input)
+{
+
+	return 0;
+}
+
+static int do_fork(void)
+{
+	char test_name[] = "test";
+
+	char *argv[] = { test_name };
+	int argc = ARRAY_SIZE(argv);
+
+	pid_t pid = fork();
+	internal_assert(pid != -1);
+
+	if (pid) {
+		int status;
+		while (waitpid(pid, &status, 0) == -1 && errno == EINTR)
+			;
+
+		return status;
+	}
+
+
+	igt_subtest_init_parse_opts(&argc, argv, short_options, long_options,
+				    "", opt_handler, NULL);
+	igt_subtest("dummy") {}
+	igt_exit();
+}
+
+int main(int argc, char **argv)
+{
+	/* no conflict */
+	long_options[0] = (struct option) { "iterations", required_argument, NULL, 'i'};
+	short_options = "";
+	internal_assert_wexited(do_fork(), 0);
+
+	/* conflict on short option */
+	long_options[0] = (struct option) { "iterations", required_argument, NULL, 'i'};
+	short_options = "h";
+	internal_assert_wsignaled(do_fork(), SIGABRT);
+
+	/* conflict on long option name */
+	long_options[0] = (struct option) { "help", required_argument, NULL, 'i'};
+	short_options = "";
+	internal_assert_wsignaled(do_fork(), SIGABRT);
+
+	/* conflict on long option 'val' representation vs short option */
+	long_options[0] = (struct option) { "iterations", required_argument, NULL, 'h'};
+	short_options = "";
+	internal_assert_wsignaled(do_fork(), SIGABRT);
+
+	/* conflict on long option 'val' representations */
+	long_options[0] = (struct option) { "iterations", required_argument, NULL, 0};
+	short_options = "";
+	internal_assert_wsignaled(do_fork(), SIGABRT);
+
+	return 0;
+}
diff --git a/lib/tests/meson.build b/lib/tests/meson.build
index 74efce39..9950bd59 100644
--- a/lib/tests/meson.build
+++ b/lib/tests/meson.build
@@ -2,6 +2,7 @@ lib_tests = [
 	'igt_assert',
 	'igt_can_fail',
 	'igt_can_fail_simple',
+	'igt_conflicting_args',
 	'igt_exit_handler',
 	'igt_fork',
 	'igt_fork_helper',