[PATCH weston 1/2] shared/option-parser: Rework option parsing
Siddharth Heroor
heroor at ti.com
Mon Jul 8 21:54:16 PDT 2013
On 7/9/2013 3:47 AM, Quentin Glidic wrote:
> From: Quentin Glidic <sardemff7+git at sardemff7.net>
>
> Long options with argument support two formats: equal ("--long=arg") and
> space ("--long arg")
> Short options now support three formats: short ("-sarg"), equal
> ("-s=arg") and space ("-s value")
>
> Provide a test program
>
> Signed-off-by: Quentin Glidic <sardemff7+git at sardemff7.net>
> ---
> man/weston.man | 4 +
> shared/option-parser.c | 111 ++++++++++++---
> tests/Makefile.am | 9 +-
> tests/option-parser-test.c | 342 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 447 insertions(+), 19 deletions(-)
> create mode 100644 tests/option-parser-test.c
>
> diff --git a/man/weston.man b/man/weston.man
> index 39d854b..79d4588 100644
> --- a/man/weston.man
> +++ b/man/weston.man
> @@ -92,6 +92,10 @@ and
> .\" ***************************************************************
> .SH OPTIONS
> .
> +Long options with an argument support two format: equal ("--long=arg") and
> +space ("--long arg"). Short options with argument support three format: short
> +("-sarg"), equal ("-s=arg") and space ("-s arg").
> +.
> .SS Weston core options:
> .TP
> \fB\-\^B\fR\fIbackend.so\fR, \fB\-\-backend\fR=\fIbackend.so\fR
> diff --git a/shared/option-parser.c b/shared/option-parser.c
> index c00349a..da9c394 100644
> --- a/shared/option-parser.c
> +++ b/shared/option-parser.c
> @@ -30,22 +30,34 @@
>
> #include "config-parser.h"
>
> -static void
> -handle_option(const struct weston_option *option, char *value)
> +enum state {
> + OK,
> + EATEN,
> + ERROR
> +};
> +
> +static enum state
> +handle_option(const struct weston_option *option, const char *value)
> {
> switch (option->type) {
> case WESTON_OPTION_INTEGER:
> + if (value == NULL || value[0] == '\0')
> + return ERROR;
> * (int32_t *) option->data = strtol(value, NULL, 0);
> - return;
> + return EATEN;
> case WESTON_OPTION_UNSIGNED_INTEGER:
> + if (value == NULL || value[0] == '\0')
> + return ERROR;
> * (uint32_t *) option->data = strtoul(value, NULL, 0);
> - return;
> + return EATEN;
Can this be a fallthrough? Looks like both of the cases use the same logic.
> case WESTON_OPTION_STRING:
> + if (value == NULL)
> + return ERROR;
> * (char **) option->data = strdup(value);
> - return;
> + return EATEN;
> case WESTON_OPTION_BOOLEAN:
> * (int32_t *) option->data = 1;
> - return;
> + return OK;
> default:
> assert(0);
> }
> @@ -56,26 +68,89 @@ parse_options(const struct weston_option *options,
> int count, int *argc, char *argv[])
> {
> int i, j, k, len = 0;
> + const char *arg, *value;
>
> for (i = 1, j = 1; i < *argc; i++) {
> - for (k = 0; k < count; k++) {
> - if (options[k].name)
> + arg = argv[i];
> + value = argv[i+1];
> +
> + if (arg[0] != '-')
> + {
> + /* Not an option, just skip it */
> + argv[j++] = argv[i];
> + continue;
> + }
> + ++arg; /* Skip the first dash */
> +
> + /* We have an option, check for which one */
> + if (arg[0] == '-') {
> + /* Long option */
> + ++arg; /* Skip the second dash */
> + for (k = 0; k < count; k++) {
> + if (!options[k].name)
> + /* No long variant for this option */
> + continue;
> +
> len = strlen(options[k].name);
> - if (options[k].name &&
> - argv[i][0] == '-' &&
> - argv[i][1] == '-' &&
> - strncmp(options[k].name, &argv[i][2], len) == 0 &&
> - (argv[i][len + 2] == '=' || argv[i][len + 2] == '\0')) {
> - handle_option(&options[k], &argv[i][len + 3]);
> + if (strncmp(options[k].name, arg, len) != 0)
> + /* Not matching */
> + continue;
> +
> + switch (arg[len]) {
> + case '=': value = arg + (len+1);
> + case '\0': break;
> + default: continue; /* Not fully matching */
> + }
> +
> + switch (handle_option(&options[k], value)) {
> + case OK: break;
> + case EATEN:
> + if (arg[len] == '\0')
> + ++i;
> + break;
> + case ERROR: return -1;
> + }
> +
> break;
> - } else if (options[k].short_name &&
> - argv[i][0] == '-' &&
> - options[k].short_name == argv[i][1]) {
> - handle_option(&options[k], &argv[i][2]);
> + }
> + } else {
> + /* Short option */
> + for (k = 0; k < count; k++) {
> + if (!options[k].short_name)
> + /* no short variant for this option */
> + continue;
> +
> + if (options[k].short_name != arg[0])
> + /* Not matching */
> + continue;
> +
> + if (arg[1] != '\0')
> + value = arg+1;
> +
> + switch (arg[1]) {
> + case '\0': break;
> + case '=': value = arg + 2; break;
> + default: value = arg + 1;
> + }
> + //fprintf(stderr, "arg = %s, value = '%s'\n", arg, value);
> +
> + switch (handle_option(&options[k], value)) {
> + case OK:
> + if (arg[1] != '\0')
> + /* Do not support merged short options */
> + return -1;
> + break;
> + case EATEN:
> + if (arg[1] == '\0')
> + ++i;
> + break;
> + case ERROR: return -1;
> + }
> break;
> }
> }
> if (k == count)
> + /* None matched */
> argv[j++] = argv[i];
> }
> argv[j] = NULL;
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index d4aa909..29ebaef 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -1,7 +1,8 @@
> TESTS = $(shared_tests) $(module_tests) $(weston_tests)
>
> shared_tests = \
> - config-parser.test
> + config-parser.test \
> + option-parser.test
>
> module_tests = \
> surface-test.la \
> @@ -57,6 +58,12 @@ config_parser_test_LDADD = \
> config_parser_test_SOURCES = \
> config-parser-test.c
>
> +option_parser_test_SOURCES = \
> + option-parser-test.c \
> + $(weston_test_runner_src)
> +option_parser_test_LDADD = \
> + ../shared/libshared.la
> +
> surface_global_test_la_SOURCES = surface-global-test.c
> surface_test_la_SOURCES = surface-test.c
>
> diff --git a/tests/option-parser-test.c b/tests/option-parser-test.c
> new file mode 100644
> index 0000000..0329122
> --- /dev/null
> +++ b/tests/option-parser-test.c
> @@ -0,0 +1,342 @@
> +/*
> + * Copyright © 2013 Quentin "Sardem FF7" Glidic
> + *
> + * Permission to use, copy, modify, distribute, and sell this software and
> + * its documentation for any purpose is hereby granted without fee, provided
> + * that the above copyright notice appear in all copies and that both that
> + * copyright notice and this permission notice appear in supporting
> + * documentation, and that the name of the copyright holders not be used in
> + * advertising or publicity pertaining to distribution of the software
> + * without specific, written prior permission. The copyright holders make
> + * no representations about the suitability of this software for any
> + * purpose. It is provided "as is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
> + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF
> + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
> + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <assert.h>
> +#include "weston-test-runner.h"
> +#include "config-parser.h"
> +
> +#define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0])
> +
> +static char *str = NULL;
> +static int32_t i = 0;
> +static uint32_t u = 0;
> +static uint8_t b = 0;
> +
> +const struct weston_option test_options[] = {
> + { WESTON_OPTION_STRING, "string", 's', &str },
> + { WESTON_OPTION_INTEGER, "int", 'i', &i },
> + { WESTON_OPTION_UNSIGNED_INTEGER, "uint", 'u', &u },
> + { WESTON_OPTION_BOOLEAN, "bool", 'b', &b },
> +};
> +
> +static void reset(void) {
> + str = NULL;
> + i = -1;
> + u = 0;
> + b = 0;
> +}
> +
> +TEST(option_parser_test_empty)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == 1);
> + assert(argc == r);
> +}
> +
> +TEST(option_parser_test_extra)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + "extra",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == 2);
> + assert(argc == r);
> +}
> +
> +TEST(option_parser_test_long_equal)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + "--string=s",
> + "--int=-10",
> + "--uint=10",
> + "--bool",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == 1);
> + assert(argc == r);
> + assert(strcmp(str, "s") == 0);
> + assert(i == -10);
> + assert(u == 10);
> + assert(b);
> +}
> +
> +TEST(option_parser_test_long_equal_extra)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + "--string=s",
> + "--int=-10",
> + "--uint=10",
> + "--bool",
> + "extra",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == 2);
> + assert(argc == r);
> + assert(strcmp(str, "s") == 0);
> + assert(i == -10);
> + assert(u == 10);
> + assert(b);
> +}
> +
> +TEST(option_parser_test_long_space)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + "--string", "s",
> + "--int", "-10",
> + "--uint", "10",
> + "--bool",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == 1);
> + assert(argc == r);
> + assert(strcmp(str, "s") == 0);
> + assert(i == -10);
> + assert(u == 10);
> + assert(b);
> +}
> +
> +TEST(option_parser_test_long_space_extra)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + "--string", "s",
> + "--int", "-10",
> + "--uint", "10",
> + "--bool",
> + "extra",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == 2);
> + assert(argc == r);
> + assert(strcmp(str, "s") == 0);
> + assert(i == -10);
> + assert(u == 10);
> + assert(b);
> +}
> +
> +TEST(option_parser_test_short)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + "-ss",
> + "-i-10",
> + "-u10",
> + "-b",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == 1);
> + assert(argc == r);
> + assert(strcmp(str, "s") == 0);
> + assert(i == -10);
> + assert(u == 10);
> + assert(b);
> +}
> +
> +TEST(option_parser_test_short_equal)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + "-s=s",
> + "-i=-10",
> + "-u=10",
> + "-b",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == 1);
> + assert(argc == r);
> + assert(strcmp(str, "s") == 0);
> + assert(i == -10);
> + assert(u == 10);
> + assert(b);
> +}
> +
> +TEST(option_parser_test_short_space)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + "-s", "s",
> + "-i", "-10",
> + "-u", "10",
> + "-b",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == 1);
> + assert(argc == r);
> + assert(strcmp(str, "s") == 0);
> + assert(i == -10);
> + assert(u == 10);
> + assert(b);
> +}
> +
> +TEST(option_parser_test_long_equal_missing_right)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + "--string=",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == 1);
> + assert(argc == r);
> + assert(strcmp(str, "") == 0);
> + assert(i == -1);
> + assert(u == 0);
> + assert(!b);
> +}
> +
> +TEST(option_parser_test_long_equal_missing_wrong)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + "--int=",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == -1);
> +}
> +
> +TEST(option_parser_test_long_space_missing)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + "--string",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == -1);
> +}
> +
> +TEST(option_parser_test_short_space_missing)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + "-s",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == -1);
> +}
> +
> +TEST(option_parser_test_short_equal_missing_right)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + "-s=",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == 1);
> + assert(argc == r);
> + assert(strcmp(str, "") == 0);
> + assert(i == -1);
> + assert(u == 0);
> + assert(!b);
> +}
> +
> +TEST(option_parser_test_short_equal_missing_wrong)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + "-i=",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == -1);
> +}
> +
> +TEST(option_parser_test_short_concat)
> +{
> + reset();
> + char *argv[] = {
> + "test",
> + "-bcd",
> + NULL
> + };
> + int argc = ARRAY_LENGTH(argv) - 1;
> +
> + int r = parse_options(test_options, ARRAY_LENGTH(test_options), &argc, argv);
> + assert(r == -1);
> +}
>
More information about the wayland-devel
mailing list