Re: [PATCH v2 1/6] qemu-option: simplify search for end of key

[Markus Armbruster <armbru@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Use strcspn to find an equal or comma value, and pass the result directly
> to get_opt_name to avoid another strchr.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  util/qemu-option.c | 35 +++++++++++++----------------------
>  1 file changed, 13 insertions(+), 22 deletions(-)
>
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index acefbc23fa..ab3b58599e 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -38,27 +38,19 @@
>  #include "qemu/help_option.h"
>  
>  /*
> - * Extracts the name of an option from the parameter string (p points at the
> + * Extracts the name of an option from the parameter string (@p points at the
>   * first byte of the option name)
>   *
> - * The option name is delimited by delim (usually , or =) or the string end
> - * and is copied into option. The caller is responsible for free'ing option
> - * when no longer required.
> + * The option name is @len characters long and is copied into @option. The
> + * caller is responsible for free'ing @option when no longer required.
>   *
>   * The return value is the position of the delimiter/zero byte after the option
> - * name in p.
> + * name in @p.
>   */
> -static const char *get_opt_name(const char *p, char **option, char delim)
> +static const char *get_opt_name(const char *p, char **option, size_t len)
>  {
> -    char *offset = strchr(p, delim);
> -
> -    if (offset) {
> -        *option = g_strndup(p, offset - p);
> -        return offset;
> -    } else {
> -        *option = g_strdup(p);
> -        return p + strlen(p);
> -    }
> +    *option = g_strndup(p, len);
> +    return p + len;
>  }

Hardly anything left; I believe this can be simplified further.  Not a
reason to delay this series.

>  
>  /*
> @@ -769,12 +761,11 @@ static const char *get_opt_name_value(const char *params,
>                                        const char *firstname,
>                                        char **name, char **value)
>  {
> -    const char *p, *pe, *pc;
> -
> -    pe = strchr(params, '=');
> -    pc = strchr(params, ',');
> +    const char *p;
> +    size_t len;
>  
> -    if (!pe || (pc && pc < pe)) {
> +    len = strcspn(params, "=,");
> +    if (params[len] != '=') {
>          /* found "foo,more" */
>          if (firstname) {
>              /* implicitly named first option */
> @@ -782,7 +773,7 @@ static const char *get_opt_name_value(const char *params,
>              p = get_opt_value(params, value);
>          } else {
>              /* option without value, must be a flag */
> -            p = get_opt_name(params, name, ',');
> +            p = get_opt_name(params, name, len);
>              if (strncmp(*name, "no", 2) == 0) {
>                  memmove(*name, *name + 2, strlen(*name + 2) + 1);
>                  *value = g_strdup("off");
> @@ -792,7 +783,7 @@ static const char *get_opt_name_value(const char *params,
>          }
>      } else {
>          /* found "foo=bar,more" */
> -        p = get_opt_name(params, name, '=');
> +        p = get_opt_name(params, name, len);
>          assert(*p == '=');
>          p++;
>          p = get_opt_value(p, value);

Reviewed-by: Markus Armbruster <armbru@redhat.com>