[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 03/10] Add an internal <secret> XML handling API



On Mon, Sep 07, 2009 at 04:12:38PM +0200, Miloslav Trmač wrote:
> Add a <secret> XML handling API, separate from the local driver, to
> avoid manually generating XML in other parts of libvirt.
> 
> * src/secret_conf.c, src/secret_conf.h: New files.
> * po/POTFILES.in, src/Makefile.am: Add secret_conf.
[...]
> +VIR_ENUM_IMPL(virSecretUsageType, VIR_SECRET_USAGE_TYPE_LAST, "none", "volume")
> +
> +void
> +virSecretDefFree(virSecretDefPtr def)
> +{
> +    if (def == NULL)
> +        return;
> +
> +    VIR_FREE(def->id);
> +    VIR_FREE(def->description);
> +    switch (def->usage_type) {
> +    case VIR_SECRET_USAGE_TYPE_NONE:
> +        break;
> +
> +    case VIR_SECRET_USAGE_TYPE_VOLUME:
> +        VIR_FREE(def->usage.volume);
> +        break;
> +
> +    default:
> +        VIR_ERROR(_("unexpected secret usage type %d"), def->usage_type);
> +        break;

  Hum, since the virSecretDefPtr is allocated by our own code, it's
  probably better to remove the default so that the compiler can tell us
  we missed one enum case if new ones gets added.

> +    }
> +    VIR_FREE(def);
> +}
> +
> +static int
> +virSecretDefParseUsage(virConnectPtr conn, xmlXPathContextPtr ctxt,
> +                       virSecretDefPtr def)
> +{
> +    char *type_str;
> +    int type;
> +
> +    type_str = virXPathString(conn, "string(./usage/@type)", ctxt);
> +    if (type_str == NULL) {
> +        virSecretReportError(conn, VIR_ERR_XML_ERROR, "%s",
> +                             _("unknown secret usage type"));

  _("missing secret usage type") would be more appropriate I guess

> +        return -1;
> +    }
> +    type = virSecretUsageTypeTypeFromString(type_str);
> +    if (type < 0) {
> +        virSecretReportError(conn, VIR_ERR_XML_ERROR,
> +                             _("unknown secret usage type %s"), type_str);
> +        VIR_FREE(type_str);
> +        return -1;
> +    }
> +    VIR_FREE(type_str);
> +    def->usage_type = type;
> +    switch (def->usage_type) {
> +    case VIR_SECRET_USAGE_TYPE_NONE:
> +        break;
> +
> +    case VIR_SECRET_USAGE_TYPE_VOLUME:
> +        def->usage.volume = virXPathString(conn, "string(./usage/volume)",
> +                                           ctxt);
> +        break;
> +
> +    default:

  Again default: here means a mismatch between
  virSecretUsageTypeTypeFromString and this function, best handled
statically IMHO.

> +        virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR,
> +                             _("unexpected secret usage type %d"),
> +                             def->usage_type);
> +        return -1;
> +    }
> +    return 0;
> +}
[...]

  But this is mostly stylistic, ACK with the error message change

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]