This looks good to me, especially with all the automatic tests added.
Quentin Glidic wrote:
-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);
Since you already checked for a zero-length string, you can detect
non-numbers with:
const char* p;
... option->data = strtol(value, 0, &p);
if (*p) return ERROR;
- 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;
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);
}
@@ -54,26 +66,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;
Good idea, leave this possibility open without deciding on it now.
+ 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;
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
