[libvirt] [PATCH] Enhance qemuParseCommandLineKeywords

[Daniel P. Berrange]

On Wed, Jan 13, 2010 at 04:48:30PM +0100, Jiri Denemark wrote:
> Current version expects name=value,... list and when an incorrect string
> such as "a,b,c=d" would be parsed as "a,b,c" keyword with "d" value
> without reporting any error, which is probably not the expected
> behavior.
> 
> This patch adds an extra argument called allowEmptyValue, which if
> non-zero will permit keywords with no value; "a,b=c,,d=" will be parsed
> as follows:
>     keyword value
>     "a"     NULL
>     "b"     "c"
>     ""      NULL
>     "d"     ""
> 
> In case allowEmptyValue is zero, the string is required to contain
> name=value pairs only; retvalues is guaranteed to contain non-NULL
> pointers. Now, "a,b,c=d" will result in an error.
> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
>  src/qemu/qemu_conf.c |   51 ++++++++++++++++++++++++++++++-------------------
>  1 files changed, 31 insertions(+), 20 deletions(-)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index d3da776..9f35217 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -3066,47 +3066,58 @@ static const char *qemuFindEnv(const char **progenv,
>  /*
>   * Takes a string containing a set of key=value,key=value,key...
>   * parameters and splits them up, returning two arrays with
> - * the individual keys and values
> + * the individual keys and values. If allowEmptyValue is nonzero,
> + * the "=value" part is optional and if a key with no value is found,
> + * NULL is be placed into corresponding place in retvalues.
>   */
>  static int
>  qemuParseCommandLineKeywords(virConnectPtr conn,
>                               const char *str,
>                               char ***retkeywords,
> -                             char ***retvalues)
> +                             char ***retvalues,
> +                             int allowEmptyValue)
>  {
>      int keywordCount = 0;
>      int keywordAlloc = 0;
>      char **keywords = NULL;
>      char **values = NULL;
>      const char *start = str;
> +    const char *end;
>      int i;
>  
>      *retkeywords = NULL;
>      *retvalues = NULL;
> +    end = start + strlen(str);
>  
>      while (start) {
>          const char *separator;
>          const char *endmark;
>          char *keyword;
> -        char *value;
> +        char *value = NULL;
>  
> -        if (!(separator = strchr(start, '='))) {
> -            qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> -                             _("malformed keyword arguments in '%s'"), str);
> -            goto error;
> +        if (!(endmark = strchr(start, ',')))
> +            endmark = end;
> +        if (!(separator = strchr(start, '=')))
> +            separator = end;
> +
> +        if (separator >= endmark) {
> +            if (!allowEmptyValue) {
> +                qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> +                                 _("malformed keyword arguments in '%s'"), str);
> +                goto error;
> +            }
> +            separator = endmark;
>          }
> +
>          if (!(keyword = strndup(start, separator - start)))
>              goto no_memory;
>  
> -        separator++;
> -        endmark = strchr(separator, ',');
> -
> -        value = endmark ?
> -            strndup(separator, endmark - separator) :
> -            strdup(separator);
> -        if (!value) {
> -            VIR_FREE(keyword);
> -            goto no_memory;
> +        if (separator < endmark) {
> +            separator++;
> +            if (!(value = strndup(separator, endmark - separator))) {
> +                VIR_FREE(keyword);
> +                goto no_memory;
> +            }
>          }
>  
>          if (keywordAlloc == keywordCount) {
> @@ -3123,7 +3134,7 @@ qemuParseCommandLineKeywords(virConnectPtr conn,
>          values[keywordCount] = value;
>          keywordCount++;
>  
> -        start = endmark ? endmark + 1 : NULL;
> +        start = endmark < end ? endmark + 1 : NULL;
>      }
>  
>      *retkeywords = keywords;
> @@ -3163,7 +3174,7 @@ qemuParseCommandLineDisk(virConnectPtr conn,
>  
>      if ((nkeywords = qemuParseCommandLineKeywords(conn, val,
>                                                    &keywords,
> -                                                  &values)) < 0)
> +                                                  &values, 0)) < 0)
>          return NULL;
>  
>      if (VIR_ALLOC(def) < 0) {
> @@ -3347,7 +3358,7 @@ qemuParseCommandLineNet(virConnectPtr conn,
>          if ((nkeywords = qemuParseCommandLineKeywords(conn,
>                                                        tmp+1,
>                                                        &keywords,
> -                                                      &values)) < 0)
> +                                                      &values, 0)) < 0)
>              return NULL;
>      } else {
>          nkeywords = 0;
> @@ -3420,7 +3431,7 @@ qemuParseCommandLineNet(virConnectPtr conn,
>          if ((nkeywords = qemuParseCommandLineKeywords(conn,
>                                                        nic + strlen("nic,"),
>                                                        &keywords,
> -                                                      &values)) < 0) {
> +                                                      &values, 0)) < 0) {
>              virDomainNetDefFree(def);
>              def = NULL;
>              goto cleanup;

ACK

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|