10:43 a.m.
On Thu, 2018-03-29 at 14:44 +0200, Pavel Hrdina wrote:
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(a)redhat.com>
> ---
> 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
All comments fixed in the second patchset.
Thanks,
Katerina