Re: [dbus PATCH 4/4] Introduce functions for translating Events to strings

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

 



On Thu, Mar 29, 2018 at 01:07:58PM +0200, Katerina Koukiou wrote:
> The functions were copied from src/util/virutil.* files from
> libvirt project.
> 
> They will be needed for other function of enum to string
> as well.

This should be split into two patches, one that introduces the
new macros and second one that uses them.

> Signed-off-by: Katerina Koukiou <kkoukiou@xxxxxxxxxx>
> ---
>  m4/virt-compile-warnings.m4 |  3 +++
>  src/events.c                | 35 +----------------------------------
>  src/util.c                  | 30 ++++++++++++++++++++++++++++++
>  src/util.h                  | 30 ++++++++++++++++++++++++++++++
>  test/test_connect.py        |  4 ++--
>  test/test_domain.py         |  8 ++++----
>  6 files changed, 70 insertions(+), 40 deletions(-)
> 
> diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
> index 6ece136..7bc49b2 100644
> --- a/m4/virt-compile-warnings.m4
> +++ b/m4/virt-compile-warnings.m4
> @@ -123,6 +123,9 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
>      # but need to rewrite various areas of code first
>      wantwarn="$wantwarn -Wno-format-truncation"
>  
> +    # Needeed for *EventToString related functions.
> +    wantwarn="$wantwarn -Wno-suggest-attribute=pure"
> +

There is no need to disable this warning, we can use G_GNUC_PURE for
the affected functions.

>      # This should be < 256 really. Currently we're down to 4096,
>      # but using 1024 bytes sized buffers (mostly for virStrerror)
>      # stops us from going down further
> diff --git a/src/events.c b/src/events.c
> index 62f3729..52c53ac 100644
> --- a/src/events.c
> +++ b/src/events.c
> @@ -12,41 +12,8 @@ virtDBusEventsDomainLifecycle(virConnectPtr connection G_GNUC_UNUSED,
>                                gpointer opaque)
>  {
>      virtDBusConnect *connect = opaque;
> -    const gchar *event_type = NULL;
>      g_autofree gchar *path = NULL;
>  
> -    switch (event) {
> -    case VIR_DOMAIN_EVENT_DEFINED:
> -        event_type = "DomainDefined";
> -        break;
> -    case VIR_DOMAIN_EVENT_UNDEFINED:
> -        event_type = "DomainUndefined";
> -        break;
> -    case VIR_DOMAIN_EVENT_STARTED:
> -        event_type = "DomainStarted";
> -        break;
> -    case VIR_DOMAIN_EVENT_SUSPENDED:
> -        event_type = "DomainSuspended";
> -        break;
> -    case VIR_DOMAIN_EVENT_RESUMED:
> -        event_type = "DomainResumed";
> -        break;
> -    case VIR_DOMAIN_EVENT_STOPPED:
> -        event_type = "DomainStopped";
> -        break;
> -    case VIR_DOMAIN_EVENT_SHUTDOWN:
> -        event_type = "DomainShutdown";
> -        break;
> -    case VIR_DOMAIN_EVENT_PMSUSPENDED:
> -        event_type = "DomainPMSuspended";
> -        break;
> -    case VIR_DOMAIN_EVENT_CRASHED:
> -        event_type = "DomainCrashed";
> -        break;
> -    default:
> -        return 0;
> -    }
> -
>      path = virtDBusUtilBusPathForVirDomain(domain, connect->domainPath);
>  
>      g_dbus_connection_emit_signal(connect->bus,
> @@ -54,7 +21,7 @@ virtDBusEventsDomainLifecycle(virConnectPtr connection G_GNUC_UNUSED,
>                                    connect->connectPath,
>                                    VIRT_DBUS_CONNECT_INTERFACE,
>                                    "Domain",
> -                                  g_variant_new("(os)", path, event_type),
> +                                  g_variant_new("(os)", path, virtDBusDomainEventToString(event)),
>                                    NULL);
>  
>      return 0;
> diff --git a/src/util.c b/src/util.c
> index d6c27f3..9f645d2 100644
> --- a/src/util.c
> +++ b/src/util.c
> @@ -124,3 +124,33 @@ virtDBusUtilVirDomainListFree(virDomainPtr *domains)
>  
>      g_free(domains);
>  }
> +
> +const gchar *virEnumToString(const gchar *const*types,
> +                             guint ntypes,
> +                             gint type)

Return type should be on a separate line, see HACKING.md and other
functions in this file.

Even though that this function is copied from libvirt where we use
vir* prefix, this package uses virtDBus* prefix and each file has
it's own prefix, so in this case the prefix is virtDBusUtil* and the
function name should be virtDBusUtilEnumToString.

> +{
> +    if (type < 0 || (unsigned)type >= ntypes)
> +        return NULL;
> +
> +    return types[type];
> +}
> +
> +VIR_ENUM_DECL(virtDBusDomainEvent)
> +VIR_ENUM_IMPL(virtDBusDomainEvent,
> +              VIR_DOMAIN_EVENT_LAST,
> +              "Defined",
> +              "Undefined",
> +              "Started",
> +              "Suspended",
> +              "Resumed",
> +              "Stopped",
> +              "Shutdown",
> +              "PMSuspended",
> +              "Crashed")

This is currently used only in event.c so it should go into top of that
file.

> +
> +const gchar *
> +virtDBusDomainEventToString(gint event)
> +{
> +    const gchar *str = virtDBusDomainEventTypeToString(event);
> +    return str ? str : "unknown";
> +}

This one as well and the function name prefix should be fixed.

> diff --git a/src/util.h b/src/util.h
> index 4304bac..22cf25e 100644
> --- a/src/util.h
> +++ b/src/util.h
> @@ -2,6 +2,7 @@
>  
>  #include "gdbus.h"
>  
> +#define VIR_ENUM_SENTINELS
>  #include <libvirt/libvirt.h>
>  
>  #define VIRT_DBUS_ERROR virtDBusErrorQuark()
> @@ -37,3 +38,32 @@ virtDBusUtilVirDomainListFree(virDomainPtr *domains);
>  
>  G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomain, virDomainFree);
>  G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainPtr, virtDBusUtilVirDomainListFree);
> +
> +gint virEnumFromString(const gchar *const*types,
> +                       guint ntypes,
> +                       const gchar *type);
> +
> +const gchar *virEnumToString(const gchar *const*types,
> +                             guint ntypes,
> +                             gint type);

Same comment for the return type and the function name.

> +
> +# define VIR_ENUM_IMPL(name, lastVal, ...) \
> +    static const gchar *const name ## TypeList[] = { __VA_ARGS__ }; \
> +    G_STATIC_ASSERT(G_N_ELEMENTS(name ## TypeList) == lastVal); \
> +    const gchar *name ## TypeToString(int type) { \
> +        return virEnumToString(name ## TypeList, \
> +                               G_N_ELEMENTS(name ## TypeList), \
> +                               type); \
> +    } \
> +    gint name ## TypeFromString(const gchar *type) { \
> +        return virEnumFromString(name ## TypeList, \
> +                                 G_N_ELEMENTS(name ## TypeList), \
> +                                 type); \
> +    }
> +
> +# define VIR_ENUM_DECL(name) \
> +    const gchar *name ## TypeToString(gint type); \
> +    gint name ## TypeFromString(const gchar*type);

There is no need to have the space after '#', that is used in libvirt
because the macro is in #ifndef ... #endif block.

Both macros should use also the VIRT_DBUS prefix instead of VIR.

Pavel

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux