[Qemu-devel] [PATCH 2/6] util: introduce glib-helper.c

Alexey Perevalov posted 6 patches 8 years, 10 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 2/6] util: introduce glib-helper.c
Posted by Alexey Perevalov 8 years, 10 months ago
There is a lack of g_int_cmp which compares pointers value in glib,
xen_disk.c introduced its own, so the same function now requires
in migration.c. So logically to move it into common place.
Futher: maybe extend glib.

Also this commit moves existing glib-compat.h into util/glib
folder for consolidation purpose.

Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
 hw/block/xen_disk.c        |  10 +-
 include/glib-compat.h      | 352 ---------------------------------------------
 include/glib/glib-compat.h | 352 +++++++++++++++++++++++++++++++++++++++++++++
 include/glib/glib-helper.h |  30 ++++
 include/qemu/osdep.h       |   2 +-
 linux-user/main.c          |   2 +-
 scripts/clean-includes     |   2 +-
 util/Makefile.objs         |   1 +
 util/glib-helper.c         |  29 ++++
 9 files changed, 417 insertions(+), 363 deletions(-)
 delete mode 100644 include/glib-compat.h
 create mode 100644 include/glib/glib-compat.h
 create mode 100644 include/glib/glib-helper.h
 create mode 100644 util/glib-helper.c

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 456a2d5..36f6396 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -20,6 +20,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "glib/glib-helper.h"
 #include <sys/ioctl.h>
 #include <sys/uio.h>
 
@@ -154,13 +155,6 @@ static void ioreq_reset(struct ioreq *ioreq)
     qemu_iovec_reset(&ioreq->v);
 }
 
-static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
-{
-    uint ua = GPOINTER_TO_UINT(a);
-    uint ub = GPOINTER_TO_UINT(b);
-    return (ua > ub) - (ua < ub);
-}
-
 static void destroy_grant(gpointer pgnt)
 {
     PersistentGrant *grant = pgnt;
@@ -1191,7 +1185,7 @@ static int blk_connect(struct XenDevice *xendev)
     if (blkdev->feature_persistent) {
         /* Init persistent grants */
         blkdev->max_grants = max_requests * BLKIF_MAX_SEGMENTS_PER_REQUEST;
-        blkdev->persistent_gnts = g_tree_new_full((GCompareDataFunc)int_cmp,
+        blkdev->persistent_gnts = g_tree_new_full((GCompareDataFunc)g_int_cmp,
                                              NULL, NULL,
                                              batch_maps ?
                                              (GDestroyNotify)g_free :
diff --git a/include/glib-compat.h b/include/glib-compat.h
deleted file mode 100644
index 863c8cf..0000000
--- a/include/glib-compat.h
+++ /dev/null
@@ -1,352 +0,0 @@
-/*
- * GLIB Compatibility Functions
- *
- * Copyright IBM, Corp. 2013
- *
- * Authors:
- *  Anthony Liguori   <aliguori@us.ibm.com>
- *  Michael Tokarev   <mjt@tls.msk.ru>
- *  Paolo Bonzini     <pbonzini@redhat.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#ifndef QEMU_GLIB_COMPAT_H
-#define QEMU_GLIB_COMPAT_H
-
-#include <glib.h>
-
-/* GLIB version compatibility flags */
-#if !GLIB_CHECK_VERSION(2, 26, 0)
-#define G_TIME_SPAN_SECOND              (G_GINT64_CONSTANT(1000000))
-#endif
-
-#if !GLIB_CHECK_VERSION(2, 28, 0)
-static inline gint64 qemu_g_get_monotonic_time(void)
-{
-    /* g_get_monotonic_time() is best-effort so we can use the wall clock as a
-     * fallback.
-     */
-
-    GTimeVal time;
-    g_get_current_time(&time);
-
-    return time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
-}
-/* work around distro backports of this interface */
-#define g_get_monotonic_time() qemu_g_get_monotonic_time()
-#endif
-
-#if defined(_WIN32) && !GLIB_CHECK_VERSION(2, 50, 0)
-/*
- * g_poll has a problem on Windows when using
- * timeouts < 10ms, so use wrapper.
- */
-#define g_poll(fds, nfds, timeout) g_poll_fixed(fds, nfds, timeout)
-gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
-#endif
-
-#if !GLIB_CHECK_VERSION(2, 30, 0)
-/* Not a 100% compatible implementation, but good enough for most
- * cases. Placeholders are only supported at the end of the
- * template. */
-static inline gchar *qemu_g_dir_make_tmp(gchar const *tmpl, GError **error)
-{
-    gchar *path = g_build_filename(g_get_tmp_dir(), tmpl ?: ".XXXXXX", NULL);
-
-    if (mkdtemp(path) != NULL) {
-        return path;
-    }
-    /* Error occurred, clean up. */
-    g_set_error(error, G_FILE_ERROR, g_file_error_from_errno(errno),
-                "mkdtemp() failed");
-    g_free(path);
-    return NULL;
-}
-#define g_dir_make_tmp(tmpl, error) qemu_g_dir_make_tmp(tmpl, error)
-#endif /* glib 2.30 */
-
-#if !GLIB_CHECK_VERSION(2, 31, 0)
-/* before glib-2.31, GMutex and GCond was dynamic-only (there was a separate
- * GStaticMutex, but it didn't work with condition variables).
- *
- * Our implementation uses GOnce to fake a static implementation that does
- * not require separate initialization.
- * We need to rename the types to avoid passing our CompatGMutex/CompatGCond
- * by mistake to a function that expects GMutex/GCond.  However, for ease
- * of use we keep the GLib function names.  GLib uses macros for the
- * implementation, we use inline functions instead and undefine the macros.
- */
-
-typedef struct CompatGMutex {
-    GOnce once;
-} CompatGMutex;
-
-typedef struct CompatGCond {
-    GOnce once;
-} CompatGCond;
-
-static inline gpointer do_g_mutex_new(gpointer unused)
-{
-    return (gpointer) g_mutex_new();
-}
-
-static inline void g_mutex_init(CompatGMutex *mutex)
-{
-    mutex->once = (GOnce) G_ONCE_INIT;
-}
-
-static inline void g_mutex_clear(CompatGMutex *mutex)
-{
-    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
-    if (mutex->once.retval) {
-        g_mutex_free((GMutex *) mutex->once.retval);
-    }
-    mutex->once = (GOnce) G_ONCE_INIT;
-}
-
-static inline void (g_mutex_lock)(CompatGMutex *mutex)
-{
-    g_once(&mutex->once, do_g_mutex_new, NULL);
-    g_mutex_lock((GMutex *) mutex->once.retval);
-}
-#undef g_mutex_lock
-
-static inline gboolean (g_mutex_trylock)(CompatGMutex *mutex)
-{
-    g_once(&mutex->once, do_g_mutex_new, NULL);
-    return g_mutex_trylock((GMutex *) mutex->once.retval);
-}
-#undef g_mutex_trylock
-
-
-static inline void (g_mutex_unlock)(CompatGMutex *mutex)
-{
-    g_mutex_unlock((GMutex *) mutex->once.retval);
-}
-#undef g_mutex_unlock
-
-static inline gpointer do_g_cond_new(gpointer unused)
-{
-    return (gpointer) g_cond_new();
-}
-
-static inline void g_cond_init(CompatGCond *cond)
-{
-    cond->once = (GOnce) G_ONCE_INIT;
-}
-
-static inline void g_cond_clear(CompatGCond *cond)
-{
-    g_assert(cond->once.status != G_ONCE_STATUS_PROGRESS);
-    if (cond->once.retval) {
-        g_cond_free((GCond *) cond->once.retval);
-    }
-    cond->once = (GOnce) G_ONCE_INIT;
-}
-
-static inline void (g_cond_wait)(CompatGCond *cond, CompatGMutex *mutex)
-{
-    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
-    g_once(&cond->once, do_g_cond_new, NULL);
-    g_cond_wait((GCond *) cond->once.retval, (GMutex *) mutex->once.retval);
-}
-#undef g_cond_wait
-
-static inline void (g_cond_broadcast)(CompatGCond *cond)
-{
-    g_once(&cond->once, do_g_cond_new, NULL);
-    g_cond_broadcast((GCond *) cond->once.retval);
-}
-#undef g_cond_broadcast
-
-static inline void (g_cond_signal)(CompatGCond *cond)
-{
-    g_once(&cond->once, do_g_cond_new, NULL);
-    g_cond_signal((GCond *) cond->once.retval);
-}
-#undef g_cond_signal
-
-static inline gboolean (g_cond_timed_wait)(CompatGCond *cond,
-                                           CompatGMutex *mutex,
-                                           GTimeVal *time)
-{
-    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
-    g_once(&cond->once, do_g_cond_new, NULL);
-    return g_cond_timed_wait((GCond *) cond->once.retval,
-                             (GMutex *) mutex->once.retval, time);
-}
-#undef g_cond_timed_wait
-
-/* This is not a macro, because it didn't exist until 2.32.  */
-static inline gboolean g_cond_wait_until(CompatGCond *cond, CompatGMutex *mutex,
-                                         gint64 end_time)
-{
-    GTimeVal time;
-
-    /* Convert from monotonic to CLOCK_REALTIME.  */
-    end_time -= g_get_monotonic_time();
-    g_get_current_time(&time);
-    end_time += time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
-
-    time.tv_sec = end_time / G_TIME_SPAN_SECOND;
-    time.tv_usec = end_time % G_TIME_SPAN_SECOND;
-    return g_cond_timed_wait(cond, mutex, &time);
-}
-
-/* before 2.31 there was no g_thread_new() */
-static inline GThread *g_thread_new(const char *name,
-                                    GThreadFunc func, gpointer data)
-{
-    GThread *thread = g_thread_create(func, data, TRUE, NULL);
-    if (!thread) {
-        g_error("creating thread");
-    }
-    return thread;
-}
-#else
-#define CompatGMutex GMutex
-#define CompatGCond GCond
-#endif /* glib 2.31 */
-
-#if !GLIB_CHECK_VERSION(2, 32, 0)
-/* Beware, function returns gboolean since 2.39.2, see GLib commit 9101915 */
-static inline void g_hash_table_add(GHashTable *hash_table, gpointer key)
-{
-    g_hash_table_replace(hash_table, key, key);
-}
-#endif
-
-#ifndef g_assert_true
-#define g_assert_true(expr)                                                    \
-    do {                                                                       \
-        if (G_LIKELY(expr)) {                                                  \
-        } else {                                                               \
-            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
-                                "'" #expr "' should be TRUE");                 \
-        }                                                                      \
-    } while (0)
-#endif
-
-#ifndef g_assert_false
-#define g_assert_false(expr)                                                   \
-    do {                                                                       \
-        if (G_LIKELY(!(expr))) {                                               \
-        } else {                                                               \
-            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
-                                "'" #expr "' should be FALSE");                \
-        }                                                                      \
-    } while (0)
-#endif
-
-#ifndef g_assert_null
-#define g_assert_null(expr)                                                    \
-    do {                                                                       \
-        if (G_LIKELY((expr) == NULL)) {                                        \
-        } else {                                                               \
-            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
-                                "'" #expr "' should be NULL");                 \
-        }                                                                      \
-    } while (0)
-#endif
-
-#ifndef g_assert_nonnull
-#define g_assert_nonnull(expr)                                                 \
-    do {                                                                       \
-        if (G_LIKELY((expr) != NULL)) {                                        \
-        } else {                                                               \
-            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
-                                "'" #expr "' should not be NULL");             \
-        }                                                                      \
-    } while (0)
-#endif
-
-#ifndef g_assert_cmpmem
-#define g_assert_cmpmem(m1, l1, m2, l2)                                        \
-    do {                                                                       \
-        gconstpointer __m1 = m1, __m2 = m2;                                    \
-        int __l1 = l1, __l2 = l2;                                              \
-        if (__l1 != __l2) {                                                    \
-            g_assertion_message_cmpnum(                                        \
-                G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,                   \
-                #l1 " (len(" #m1 ")) == " #l2 " (len(" #m2 "))", __l1, "==",   \
-                __l2, 'i');                                                    \
-        } else if (memcmp(__m1, __m2, __l1) != 0) {                            \
-            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
-                                "assertion failed (" #m1 " == " #m2 ")");      \
-        }                                                                      \
-    } while (0)
-#endif
-
-#if !GLIB_CHECK_VERSION(2, 28, 0)
-static inline void g_list_free_full(GList *list, GDestroyNotify free_func)
-{
-    GList *l;
-
-    for (l = list; l; l = l->next) {
-        free_func(l->data);
-    }
-
-    g_list_free(list);
-}
-
-static inline void g_slist_free_full(GSList *list, GDestroyNotify free_func)
-{
-    GSList *l;
-
-    for (l = list; l; l = l->next) {
-        free_func(l->data);
-    }
-
-    g_slist_free(list);
-}
-#endif
-
-#if !GLIB_CHECK_VERSION(2, 26, 0)
-static inline void g_source_set_name(GSource *source, const char *name)
-{
-    /* This is just a debugging aid, so leaving it a no-op */
-}
-static inline void g_source_set_name_by_id(guint tag, const char *name)
-{
-    /* This is just a debugging aid, so leaving it a no-op */
-}
-#endif
-
-#if !GLIB_CHECK_VERSION(2, 36, 0)
-/* Always fail.  This will not include error_report output in the test log,
- * sending it instead to stderr.
- */
-#define g_test_initialized() (0)
-#endif
-#if !GLIB_CHECK_VERSION(2, 38, 0)
-#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
-#error schizophrenic detection of glib subprocess testing
-#endif
-#define g_test_subprocess() (0)
-#endif
-
-
-#if !GLIB_CHECK_VERSION(2, 34, 0)
-static inline void
-g_test_add_data_func_full(const char *path,
-                          gpointer data,
-                          gpointer fn,
-                          gpointer data_free_func)
-{
-#if GLIB_CHECK_VERSION(2, 26, 0)
-    /* back-compat casts, remove this once we can require new-enough glib */
-    g_test_add_vtable(path, 0, data, NULL,
-                      (GTestFixtureFunc)fn, (GTestFixtureFunc) data_free_func);
-#else
-    /* back-compat casts, remove this once we can require new-enough glib */
-    g_test_add_vtable(path, 0, data, NULL,
-                      (void (*)(void)) fn, (void (*)(void)) data_free_func);
-#endif
-}
-#endif
-
-
-#endif
diff --git a/include/glib/glib-compat.h b/include/glib/glib-compat.h
new file mode 100644
index 0000000..863c8cf
--- /dev/null
+++ b/include/glib/glib-compat.h
@@ -0,0 +1,352 @@
+/*
+ * GLIB Compatibility Functions
+ *
+ * Copyright IBM, Corp. 2013
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *  Michael Tokarev   <mjt@tls.msk.ru>
+ *  Paolo Bonzini     <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_GLIB_COMPAT_H
+#define QEMU_GLIB_COMPAT_H
+
+#include <glib.h>
+
+/* GLIB version compatibility flags */
+#if !GLIB_CHECK_VERSION(2, 26, 0)
+#define G_TIME_SPAN_SECOND              (G_GINT64_CONSTANT(1000000))
+#endif
+
+#if !GLIB_CHECK_VERSION(2, 28, 0)
+static inline gint64 qemu_g_get_monotonic_time(void)
+{
+    /* g_get_monotonic_time() is best-effort so we can use the wall clock as a
+     * fallback.
+     */
+
+    GTimeVal time;
+    g_get_current_time(&time);
+
+    return time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
+}
+/* work around distro backports of this interface */
+#define g_get_monotonic_time() qemu_g_get_monotonic_time()
+#endif
+
+#if defined(_WIN32) && !GLIB_CHECK_VERSION(2, 50, 0)
+/*
+ * g_poll has a problem on Windows when using
+ * timeouts < 10ms, so use wrapper.
+ */
+#define g_poll(fds, nfds, timeout) g_poll_fixed(fds, nfds, timeout)
+gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
+#endif
+
+#if !GLIB_CHECK_VERSION(2, 30, 0)
+/* Not a 100% compatible implementation, but good enough for most
+ * cases. Placeholders are only supported at the end of the
+ * template. */
+static inline gchar *qemu_g_dir_make_tmp(gchar const *tmpl, GError **error)
+{
+    gchar *path = g_build_filename(g_get_tmp_dir(), tmpl ?: ".XXXXXX", NULL);
+
+    if (mkdtemp(path) != NULL) {
+        return path;
+    }
+    /* Error occurred, clean up. */
+    g_set_error(error, G_FILE_ERROR, g_file_error_from_errno(errno),
+                "mkdtemp() failed");
+    g_free(path);
+    return NULL;
+}
+#define g_dir_make_tmp(tmpl, error) qemu_g_dir_make_tmp(tmpl, error)
+#endif /* glib 2.30 */
+
+#if !GLIB_CHECK_VERSION(2, 31, 0)
+/* before glib-2.31, GMutex and GCond was dynamic-only (there was a separate
+ * GStaticMutex, but it didn't work with condition variables).
+ *
+ * Our implementation uses GOnce to fake a static implementation that does
+ * not require separate initialization.
+ * We need to rename the types to avoid passing our CompatGMutex/CompatGCond
+ * by mistake to a function that expects GMutex/GCond.  However, for ease
+ * of use we keep the GLib function names.  GLib uses macros for the
+ * implementation, we use inline functions instead and undefine the macros.
+ */
+
+typedef struct CompatGMutex {
+    GOnce once;
+} CompatGMutex;
+
+typedef struct CompatGCond {
+    GOnce once;
+} CompatGCond;
+
+static inline gpointer do_g_mutex_new(gpointer unused)
+{
+    return (gpointer) g_mutex_new();
+}
+
+static inline void g_mutex_init(CompatGMutex *mutex)
+{
+    mutex->once = (GOnce) G_ONCE_INIT;
+}
+
+static inline void g_mutex_clear(CompatGMutex *mutex)
+{
+    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
+    if (mutex->once.retval) {
+        g_mutex_free((GMutex *) mutex->once.retval);
+    }
+    mutex->once = (GOnce) G_ONCE_INIT;
+}
+
+static inline void (g_mutex_lock)(CompatGMutex *mutex)
+{
+    g_once(&mutex->once, do_g_mutex_new, NULL);
+    g_mutex_lock((GMutex *) mutex->once.retval);
+}
+#undef g_mutex_lock
+
+static inline gboolean (g_mutex_trylock)(CompatGMutex *mutex)
+{
+    g_once(&mutex->once, do_g_mutex_new, NULL);
+    return g_mutex_trylock((GMutex *) mutex->once.retval);
+}
+#undef g_mutex_trylock
+
+
+static inline void (g_mutex_unlock)(CompatGMutex *mutex)
+{
+    g_mutex_unlock((GMutex *) mutex->once.retval);
+}
+#undef g_mutex_unlock
+
+static inline gpointer do_g_cond_new(gpointer unused)
+{
+    return (gpointer) g_cond_new();
+}
+
+static inline void g_cond_init(CompatGCond *cond)
+{
+    cond->once = (GOnce) G_ONCE_INIT;
+}
+
+static inline void g_cond_clear(CompatGCond *cond)
+{
+    g_assert(cond->once.status != G_ONCE_STATUS_PROGRESS);
+    if (cond->once.retval) {
+        g_cond_free((GCond *) cond->once.retval);
+    }
+    cond->once = (GOnce) G_ONCE_INIT;
+}
+
+static inline void (g_cond_wait)(CompatGCond *cond, CompatGMutex *mutex)
+{
+    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
+    g_once(&cond->once, do_g_cond_new, NULL);
+    g_cond_wait((GCond *) cond->once.retval, (GMutex *) mutex->once.retval);
+}
+#undef g_cond_wait
+
+static inline void (g_cond_broadcast)(CompatGCond *cond)
+{
+    g_once(&cond->once, do_g_cond_new, NULL);
+    g_cond_broadcast((GCond *) cond->once.retval);
+}
+#undef g_cond_broadcast
+
+static inline void (g_cond_signal)(CompatGCond *cond)
+{
+    g_once(&cond->once, do_g_cond_new, NULL);
+    g_cond_signal((GCond *) cond->once.retval);
+}
+#undef g_cond_signal
+
+static inline gboolean (g_cond_timed_wait)(CompatGCond *cond,
+                                           CompatGMutex *mutex,
+                                           GTimeVal *time)
+{
+    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
+    g_once(&cond->once, do_g_cond_new, NULL);
+    return g_cond_timed_wait((GCond *) cond->once.retval,
+                             (GMutex *) mutex->once.retval, time);
+}
+#undef g_cond_timed_wait
+
+/* This is not a macro, because it didn't exist until 2.32.  */
+static inline gboolean g_cond_wait_until(CompatGCond *cond, CompatGMutex *mutex,
+                                         gint64 end_time)
+{
+    GTimeVal time;
+
+    /* Convert from monotonic to CLOCK_REALTIME.  */
+    end_time -= g_get_monotonic_time();
+    g_get_current_time(&time);
+    end_time += time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
+
+    time.tv_sec = end_time / G_TIME_SPAN_SECOND;
+    time.tv_usec = end_time % G_TIME_SPAN_SECOND;
+    return g_cond_timed_wait(cond, mutex, &time);
+}
+
+/* before 2.31 there was no g_thread_new() */
+static inline GThread *g_thread_new(const char *name,
+                                    GThreadFunc func, gpointer data)
+{
+    GThread *thread = g_thread_create(func, data, TRUE, NULL);
+    if (!thread) {
+        g_error("creating thread");
+    }
+    return thread;
+}
+#else
+#define CompatGMutex GMutex
+#define CompatGCond GCond
+#endif /* glib 2.31 */
+
+#if !GLIB_CHECK_VERSION(2, 32, 0)
+/* Beware, function returns gboolean since 2.39.2, see GLib commit 9101915 */
+static inline void g_hash_table_add(GHashTable *hash_table, gpointer key)
+{
+    g_hash_table_replace(hash_table, key, key);
+}
+#endif
+
+#ifndef g_assert_true
+#define g_assert_true(expr)                                                    \
+    do {                                                                       \
+        if (G_LIKELY(expr)) {                                                  \
+        } else {                                                               \
+            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
+                                "'" #expr "' should be TRUE");                 \
+        }                                                                      \
+    } while (0)
+#endif
+
+#ifndef g_assert_false
+#define g_assert_false(expr)                                                   \
+    do {                                                                       \
+        if (G_LIKELY(!(expr))) {                                               \
+        } else {                                                               \
+            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
+                                "'" #expr "' should be FALSE");                \
+        }                                                                      \
+    } while (0)
+#endif
+
+#ifndef g_assert_null
+#define g_assert_null(expr)                                                    \
+    do {                                                                       \
+        if (G_LIKELY((expr) == NULL)) {                                        \
+        } else {                                                               \
+            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
+                                "'" #expr "' should be NULL");                 \
+        }                                                                      \
+    } while (0)
+#endif
+
+#ifndef g_assert_nonnull
+#define g_assert_nonnull(expr)                                                 \
+    do {                                                                       \
+        if (G_LIKELY((expr) != NULL)) {                                        \
+        } else {                                                               \
+            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
+                                "'" #expr "' should not be NULL");             \
+        }                                                                      \
+    } while (0)
+#endif
+
+#ifndef g_assert_cmpmem
+#define g_assert_cmpmem(m1, l1, m2, l2)                                        \
+    do {                                                                       \
+        gconstpointer __m1 = m1, __m2 = m2;                                    \
+        int __l1 = l1, __l2 = l2;                                              \
+        if (__l1 != __l2) {                                                    \
+            g_assertion_message_cmpnum(                                        \
+                G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,                   \
+                #l1 " (len(" #m1 ")) == " #l2 " (len(" #m2 "))", __l1, "==",   \
+                __l2, 'i');                                                    \
+        } else if (memcmp(__m1, __m2, __l1) != 0) {                            \
+            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
+                                "assertion failed (" #m1 " == " #m2 ")");      \
+        }                                                                      \
+    } while (0)
+#endif
+
+#if !GLIB_CHECK_VERSION(2, 28, 0)
+static inline void g_list_free_full(GList *list, GDestroyNotify free_func)
+{
+    GList *l;
+
+    for (l = list; l; l = l->next) {
+        free_func(l->data);
+    }
+
+    g_list_free(list);
+}
+
+static inline void g_slist_free_full(GSList *list, GDestroyNotify free_func)
+{
+    GSList *l;
+
+    for (l = list; l; l = l->next) {
+        free_func(l->data);
+    }
+
+    g_slist_free(list);
+}
+#endif
+
+#if !GLIB_CHECK_VERSION(2, 26, 0)
+static inline void g_source_set_name(GSource *source, const char *name)
+{
+    /* This is just a debugging aid, so leaving it a no-op */
+}
+static inline void g_source_set_name_by_id(guint tag, const char *name)
+{
+    /* This is just a debugging aid, so leaving it a no-op */
+}
+#endif
+
+#if !GLIB_CHECK_VERSION(2, 36, 0)
+/* Always fail.  This will not include error_report output in the test log,
+ * sending it instead to stderr.
+ */
+#define g_test_initialized() (0)
+#endif
+#if !GLIB_CHECK_VERSION(2, 38, 0)
+#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
+#error schizophrenic detection of glib subprocess testing
+#endif
+#define g_test_subprocess() (0)
+#endif
+
+
+#if !GLIB_CHECK_VERSION(2, 34, 0)
+static inline void
+g_test_add_data_func_full(const char *path,
+                          gpointer data,
+                          gpointer fn,
+                          gpointer data_free_func)
+{
+#if GLIB_CHECK_VERSION(2, 26, 0)
+    /* back-compat casts, remove this once we can require new-enough glib */
+    g_test_add_vtable(path, 0, data, NULL,
+                      (GTestFixtureFunc)fn, (GTestFixtureFunc) data_free_func);
+#else
+    /* back-compat casts, remove this once we can require new-enough glib */
+    g_test_add_vtable(path, 0, data, NULL,
+                      (void (*)(void)) fn, (void (*)(void)) data_free_func);
+#endif
+}
+#endif
+
+
+#endif
diff --git a/include/glib/glib-helper.h b/include/glib/glib-helper.h
new file mode 100644
index 0000000..db740fb
--- /dev/null
+++ b/include/glib/glib-helper.h
@@ -0,0 +1,30 @@
+/*
+ * Helpers for GLIB
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_GLIB_HELPER_H
+#define QEMU_GLIB_HELPER_H
+
+
+#include "glib/glib-compat.h"
+
+#define GPOINTER_TO_UINT64(a) ((guint64) (a))
+
+/*
+ * return 1 in case of a > b, -1 otherwise and 0 if equeal
+ */
+gint g_int_cmp64(gconstpointer a, gconstpointer b,
+        gpointer __attribute__((unused)) user_data);
+
+/*
+ * return 1 in case of a > b, -1 otherwise and 0 if equeal
+ */
+int g_int_cmp(gconstpointer a, gconstpointer b,
+        gpointer __attribute__((unused)) user_data);
+
+#endif /* QEMU_GLIB_HELPER_H */
+
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 122ff06..36f8a89 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -104,7 +104,7 @@ extern int daemon(int, int);
 #include "sysemu/os-posix.h"
 #endif
 
-#include "glib-compat.h"
+#include "glib/glib-compat.h"
 #include "qemu/typedefs.h"
 
 #ifndef O_LARGEFILE
diff --git a/linux-user/main.c b/linux-user/main.c
index 10a3bb3..7cea6bc 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -35,7 +35,7 @@
 #include "elf.h"
 #include "exec/log.h"
 #include "trace/control.h"
-#include "glib-compat.h"
+#include "glib/glib-compat.h"
 
 char *exec_path;
 
diff --git a/scripts/clean-includes b/scripts/clean-includes
index dd938da..b32b928 100755
--- a/scripts/clean-includes
+++ b/scripts/clean-includes
@@ -123,7 +123,7 @@ for f in "$@"; do
       ;;
     *include/qemu/osdep.h | \
     *include/qemu/compiler.h | \
-    *include/glib-compat.h | \
+    *include/glib/glib-compat.h | \
     *include/sysemu/os-posix.h | \
     *include/sysemu/os-win32.h | \
     *include/standard-headers/ )
diff --git a/util/Makefile.objs b/util/Makefile.objs
index c6205eb..0080712 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -43,3 +43,4 @@ util-obj-y += qdist.o
 util-obj-y += qht.o
 util-obj-y += range.o
 util-obj-y += systemd.o
+util-obj-y += glib-helper.o
diff --git a/util/glib-helper.c b/util/glib-helper.c
new file mode 100644
index 0000000..2557009
--- /dev/null
+++ b/util/glib-helper.c
@@ -0,0 +1,29 @@
+/*
+ * Implementation for GLIB helpers
+ * this file is intented to commulate and later reuse
+ * additional glib functions
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+
+ */
+
+#include "glib/glib-helper.h"
+
+gint g_int_cmp64(gconstpointer a, gconstpointer b,
+        gpointer __attribute__((unused)) user_data)
+{
+    guint64 ua = GPOINTER_TO_UINT64(a);
+    guint64 ub = GPOINTER_TO_UINT64(b);
+    return (ua > ub) - (ua < ub);
+}
+
+/*
+ * return 1 in case of a > b, -1 otherwise and 0 if equeal
+ */
+gint g_int_cmp(gconstpointer a, gconstpointer b,
+        gpointer __attribute__((unused)) user_data)
+{
+    return g_int_cmp64(a, b, user_data);
+}
+
-- 
1.8.3.1


Re: [Qemu-devel] [PATCH 2/6] util: introduce glib-helper.c
Posted by Philippe Mathieu-Daudé 8 years, 10 months ago
Hi Alexey,

On 04/14/2017 10:17 AM, Alexey Perevalov wrote:
> There is a lack of g_int_cmp which compares pointers value in glib,
> xen_disk.c introduced its own, so the same function now requires
> in migration.c. So logically to move it into common place.
> Futher: maybe extend glib.
>
> Also this commit moves existing glib-compat.h into util/glib
> folder for consolidation purpose.

Can you do this in two commits? First one moving files only, second move 
the function?

I'm not sure naming it "g_int_cmp()" won't clash with future _extended_ 
glib, what do you think about naming it "qemu_g_int_cmp()"?

> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> ---
>  hw/block/xen_disk.c        |  10 +-
>  include/glib-compat.h      | 352 ---------------------------------------------
>  include/glib/glib-compat.h | 352 +++++++++++++++++++++++++++++++++++++++++++++
>  include/glib/glib-helper.h |  30 ++++
>  include/qemu/osdep.h       |   2 +-
>  linux-user/main.c          |   2 +-
>  scripts/clean-includes     |   2 +-
>  util/Makefile.objs         |   1 +
>  util/glib-helper.c         |  29 ++++
>  9 files changed, 417 insertions(+), 363 deletions(-)
>  delete mode 100644 include/glib-compat.h
>  create mode 100644 include/glib/glib-compat.h
>  create mode 100644 include/glib/glib-helper.h
>  create mode 100644 util/glib-helper.c
>
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 456a2d5..36f6396 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -20,6 +20,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "glib/glib-helper.h"
>  #include <sys/ioctl.h>
>  #include <sys/uio.h>
>
> @@ -154,13 +155,6 @@ static void ioreq_reset(struct ioreq *ioreq)
>      qemu_iovec_reset(&ioreq->v);
>  }
>
> -static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
> -{
> -    uint ua = GPOINTER_TO_UINT(a);
> -    uint ub = GPOINTER_TO_UINT(b);
> -    return (ua > ub) - (ua < ub);
> -}
> -
>  static void destroy_grant(gpointer pgnt)
>  {
>      PersistentGrant *grant = pgnt;
> @@ -1191,7 +1185,7 @@ static int blk_connect(struct XenDevice *xendev)
>      if (blkdev->feature_persistent) {
>          /* Init persistent grants */
>          blkdev->max_grants = max_requests * BLKIF_MAX_SEGMENTS_PER_REQUEST;
> -        blkdev->persistent_gnts = g_tree_new_full((GCompareDataFunc)int_cmp,
> +        blkdev->persistent_gnts = g_tree_new_full((GCompareDataFunc)g_int_cmp,
>                                               NULL, NULL,
>                                               batch_maps ?
>                                               (GDestroyNotify)g_free :
> diff --git a/include/glib-compat.h b/include/glib-compat.h
> deleted file mode 100644
> index 863c8cf..0000000
> --- a/include/glib-compat.h
> +++ /dev/null
> @@ -1,352 +0,0 @@
> -/*
> - * GLIB Compatibility Functions
> - *
> - * Copyright IBM, Corp. 2013
> - *
> - * Authors:
> - *  Anthony Liguori   <aliguori@us.ibm.com>
> - *  Michael Tokarev   <mjt@tls.msk.ru>
> - *  Paolo Bonzini     <pbonzini@redhat.com>
> - *
> - * This work is licensed under the terms of the GNU GPL, version 2 or later.
> - * See the COPYING file in the top-level directory.
> - *
> - */
> -
> -#ifndef QEMU_GLIB_COMPAT_H
> -#define QEMU_GLIB_COMPAT_H
> -
> -#include <glib.h>
> -
> -/* GLIB version compatibility flags */
> -#if !GLIB_CHECK_VERSION(2, 26, 0)
> -#define G_TIME_SPAN_SECOND              (G_GINT64_CONSTANT(1000000))
> -#endif
> -
> -#if !GLIB_CHECK_VERSION(2, 28, 0)
> -static inline gint64 qemu_g_get_monotonic_time(void)
> -{
> -    /* g_get_monotonic_time() is best-effort so we can use the wall clock as a
> -     * fallback.
> -     */
> -
> -    GTimeVal time;
> -    g_get_current_time(&time);
> -
> -    return time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
> -}
> -/* work around distro backports of this interface */
> -#define g_get_monotonic_time() qemu_g_get_monotonic_time()
> -#endif
> -
> -#if defined(_WIN32) && !GLIB_CHECK_VERSION(2, 50, 0)
> -/*
> - * g_poll has a problem on Windows when using
> - * timeouts < 10ms, so use wrapper.
> - */
> -#define g_poll(fds, nfds, timeout) g_poll_fixed(fds, nfds, timeout)
> -gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
> -#endif
> -
> -#if !GLIB_CHECK_VERSION(2, 30, 0)
> -/* Not a 100% compatible implementation, but good enough for most
> - * cases. Placeholders are only supported at the end of the
> - * template. */
> -static inline gchar *qemu_g_dir_make_tmp(gchar const *tmpl, GError **error)
> -{
> -    gchar *path = g_build_filename(g_get_tmp_dir(), tmpl ?: ".XXXXXX", NULL);
> -
> -    if (mkdtemp(path) != NULL) {
> -        return path;
> -    }
> -    /* Error occurred, clean up. */
> -    g_set_error(error, G_FILE_ERROR, g_file_error_from_errno(errno),
> -                "mkdtemp() failed");
> -    g_free(path);
> -    return NULL;
> -}
> -#define g_dir_make_tmp(tmpl, error) qemu_g_dir_make_tmp(tmpl, error)
> -#endif /* glib 2.30 */
> -
> -#if !GLIB_CHECK_VERSION(2, 31, 0)
> -/* before glib-2.31, GMutex and GCond was dynamic-only (there was a separate
> - * GStaticMutex, but it didn't work with condition variables).
> - *
> - * Our implementation uses GOnce to fake a static implementation that does
> - * not require separate initialization.
> - * We need to rename the types to avoid passing our CompatGMutex/CompatGCond
> - * by mistake to a function that expects GMutex/GCond.  However, for ease
> - * of use we keep the GLib function names.  GLib uses macros for the
> - * implementation, we use inline functions instead and undefine the macros.
> - */
> -
> -typedef struct CompatGMutex {
> -    GOnce once;
> -} CompatGMutex;
> -
> -typedef struct CompatGCond {
> -    GOnce once;
> -} CompatGCond;
> -
> -static inline gpointer do_g_mutex_new(gpointer unused)
> -{
> -    return (gpointer) g_mutex_new();
> -}
> -
> -static inline void g_mutex_init(CompatGMutex *mutex)
> -{
> -    mutex->once = (GOnce) G_ONCE_INIT;
> -}
> -
> -static inline void g_mutex_clear(CompatGMutex *mutex)
> -{
> -    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> -    if (mutex->once.retval) {
> -        g_mutex_free((GMutex *) mutex->once.retval);
> -    }
> -    mutex->once = (GOnce) G_ONCE_INIT;
> -}
> -
> -static inline void (g_mutex_lock)(CompatGMutex *mutex)
> -{
> -    g_once(&mutex->once, do_g_mutex_new, NULL);
> -    g_mutex_lock((GMutex *) mutex->once.retval);
> -}
> -#undef g_mutex_lock
> -
> -static inline gboolean (g_mutex_trylock)(CompatGMutex *mutex)
> -{
> -    g_once(&mutex->once, do_g_mutex_new, NULL);
> -    return g_mutex_trylock((GMutex *) mutex->once.retval);
> -}
> -#undef g_mutex_trylock
> -
> -
> -static inline void (g_mutex_unlock)(CompatGMutex *mutex)
> -{
> -    g_mutex_unlock((GMutex *) mutex->once.retval);
> -}
> -#undef g_mutex_unlock
> -
> -static inline gpointer do_g_cond_new(gpointer unused)
> -{
> -    return (gpointer) g_cond_new();
> -}
> -
> -static inline void g_cond_init(CompatGCond *cond)
> -{
> -    cond->once = (GOnce) G_ONCE_INIT;
> -}
> -
> -static inline void g_cond_clear(CompatGCond *cond)
> -{
> -    g_assert(cond->once.status != G_ONCE_STATUS_PROGRESS);
> -    if (cond->once.retval) {
> -        g_cond_free((GCond *) cond->once.retval);
> -    }
> -    cond->once = (GOnce) G_ONCE_INIT;
> -}
> -
> -static inline void (g_cond_wait)(CompatGCond *cond, CompatGMutex *mutex)
> -{
> -    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> -    g_once(&cond->once, do_g_cond_new, NULL);
> -    g_cond_wait((GCond *) cond->once.retval, (GMutex *) mutex->once.retval);
> -}
> -#undef g_cond_wait
> -
> -static inline void (g_cond_broadcast)(CompatGCond *cond)
> -{
> -    g_once(&cond->once, do_g_cond_new, NULL);
> -    g_cond_broadcast((GCond *) cond->once.retval);
> -}
> -#undef g_cond_broadcast
> -
> -static inline void (g_cond_signal)(CompatGCond *cond)
> -{
> -    g_once(&cond->once, do_g_cond_new, NULL);
> -    g_cond_signal((GCond *) cond->once.retval);
> -}
> -#undef g_cond_signal
> -
> -static inline gboolean (g_cond_timed_wait)(CompatGCond *cond,
> -                                           CompatGMutex *mutex,
> -                                           GTimeVal *time)
> -{
> -    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> -    g_once(&cond->once, do_g_cond_new, NULL);
> -    return g_cond_timed_wait((GCond *) cond->once.retval,
> -                             (GMutex *) mutex->once.retval, time);
> -}
> -#undef g_cond_timed_wait
> -
> -/* This is not a macro, because it didn't exist until 2.32.  */
> -static inline gboolean g_cond_wait_until(CompatGCond *cond, CompatGMutex *mutex,
> -                                         gint64 end_time)
> -{
> -    GTimeVal time;
> -
> -    /* Convert from monotonic to CLOCK_REALTIME.  */
> -    end_time -= g_get_monotonic_time();
> -    g_get_current_time(&time);
> -    end_time += time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
> -
> -    time.tv_sec = end_time / G_TIME_SPAN_SECOND;
> -    time.tv_usec = end_time % G_TIME_SPAN_SECOND;
> -    return g_cond_timed_wait(cond, mutex, &time);
> -}
> -
> -/* before 2.31 there was no g_thread_new() */
> -static inline GThread *g_thread_new(const char *name,
> -                                    GThreadFunc func, gpointer data)
> -{
> -    GThread *thread = g_thread_create(func, data, TRUE, NULL);
> -    if (!thread) {
> -        g_error("creating thread");
> -    }
> -    return thread;
> -}
> -#else
> -#define CompatGMutex GMutex
> -#define CompatGCond GCond
> -#endif /* glib 2.31 */
> -
> -#if !GLIB_CHECK_VERSION(2, 32, 0)
> -/* Beware, function returns gboolean since 2.39.2, see GLib commit 9101915 */
> -static inline void g_hash_table_add(GHashTable *hash_table, gpointer key)
> -{
> -    g_hash_table_replace(hash_table, key, key);
> -}
> -#endif
> -
> -#ifndef g_assert_true
> -#define g_assert_true(expr)                                                    \
> -    do {                                                                       \
> -        if (G_LIKELY(expr)) {                                                  \
> -        } else {                                                               \
> -            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> -                                "'" #expr "' should be TRUE");                 \
> -        }                                                                      \
> -    } while (0)
> -#endif
> -
> -#ifndef g_assert_false
> -#define g_assert_false(expr)                                                   \
> -    do {                                                                       \
> -        if (G_LIKELY(!(expr))) {                                               \
> -        } else {                                                               \
> -            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> -                                "'" #expr "' should be FALSE");                \
> -        }                                                                      \
> -    } while (0)
> -#endif
> -
> -#ifndef g_assert_null
> -#define g_assert_null(expr)                                                    \
> -    do {                                                                       \
> -        if (G_LIKELY((expr) == NULL)) {                                        \
> -        } else {                                                               \
> -            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> -                                "'" #expr "' should be NULL");                 \
> -        }                                                                      \
> -    } while (0)
> -#endif
> -
> -#ifndef g_assert_nonnull
> -#define g_assert_nonnull(expr)                                                 \
> -    do {                                                                       \
> -        if (G_LIKELY((expr) != NULL)) {                                        \
> -        } else {                                                               \
> -            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> -                                "'" #expr "' should not be NULL");             \
> -        }                                                                      \
> -    } while (0)
> -#endif
> -
> -#ifndef g_assert_cmpmem
> -#define g_assert_cmpmem(m1, l1, m2, l2)                                        \
> -    do {                                                                       \
> -        gconstpointer __m1 = m1, __m2 = m2;                                    \
> -        int __l1 = l1, __l2 = l2;                                              \
> -        if (__l1 != __l2) {                                                    \
> -            g_assertion_message_cmpnum(                                        \
> -                G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,                   \
> -                #l1 " (len(" #m1 ")) == " #l2 " (len(" #m2 "))", __l1, "==",   \
> -                __l2, 'i');                                                    \
> -        } else if (memcmp(__m1, __m2, __l1) != 0) {                            \
> -            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> -                                "assertion failed (" #m1 " == " #m2 ")");      \
> -        }                                                                      \
> -    } while (0)
> -#endif
> -
> -#if !GLIB_CHECK_VERSION(2, 28, 0)
> -static inline void g_list_free_full(GList *list, GDestroyNotify free_func)
> -{
> -    GList *l;
> -
> -    for (l = list; l; l = l->next) {
> -        free_func(l->data);
> -    }
> -
> -    g_list_free(list);
> -}
> -
> -static inline void g_slist_free_full(GSList *list, GDestroyNotify free_func)
> -{
> -    GSList *l;
> -
> -    for (l = list; l; l = l->next) {
> -        free_func(l->data);
> -    }
> -
> -    g_slist_free(list);
> -}
> -#endif
> -
> -#if !GLIB_CHECK_VERSION(2, 26, 0)
> -static inline void g_source_set_name(GSource *source, const char *name)
> -{
> -    /* This is just a debugging aid, so leaving it a no-op */
> -}
> -static inline void g_source_set_name_by_id(guint tag, const char *name)
> -{
> -    /* This is just a debugging aid, so leaving it a no-op */
> -}
> -#endif
> -
> -#if !GLIB_CHECK_VERSION(2, 36, 0)
> -/* Always fail.  This will not include error_report output in the test log,
> - * sending it instead to stderr.
> - */
> -#define g_test_initialized() (0)
> -#endif
> -#if !GLIB_CHECK_VERSION(2, 38, 0)
> -#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
> -#error schizophrenic detection of glib subprocess testing
> -#endif
> -#define g_test_subprocess() (0)
> -#endif
> -
> -
> -#if !GLIB_CHECK_VERSION(2, 34, 0)
> -static inline void
> -g_test_add_data_func_full(const char *path,
> -                          gpointer data,
> -                          gpointer fn,
> -                          gpointer data_free_func)
> -{
> -#if GLIB_CHECK_VERSION(2, 26, 0)
> -    /* back-compat casts, remove this once we can require new-enough glib */
> -    g_test_add_vtable(path, 0, data, NULL,
> -                      (GTestFixtureFunc)fn, (GTestFixtureFunc) data_free_func);
> -#else
> -    /* back-compat casts, remove this once we can require new-enough glib */
> -    g_test_add_vtable(path, 0, data, NULL,
> -                      (void (*)(void)) fn, (void (*)(void)) data_free_func);
> -#endif
> -}
> -#endif
> -
> -
> -#endif
> diff --git a/include/glib/glib-compat.h b/include/glib/glib-compat.h
> new file mode 100644
> index 0000000..863c8cf
> --- /dev/null
> +++ b/include/glib/glib-compat.h
> @@ -0,0 +1,352 @@
> +/*
> + * GLIB Compatibility Functions
> + *
> + * Copyright IBM, Corp. 2013
> + *
> + * Authors:
> + *  Anthony Liguori   <aliguori@us.ibm.com>
> + *  Michael Tokarev   <mjt@tls.msk.ru>
> + *  Paolo Bonzini     <pbonzini@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef QEMU_GLIB_COMPAT_H
> +#define QEMU_GLIB_COMPAT_H
> +
> +#include <glib.h>
> +
> +/* GLIB version compatibility flags */
> +#if !GLIB_CHECK_VERSION(2, 26, 0)
> +#define G_TIME_SPAN_SECOND              (G_GINT64_CONSTANT(1000000))
> +#endif
> +
> +#if !GLIB_CHECK_VERSION(2, 28, 0)
> +static inline gint64 qemu_g_get_monotonic_time(void)
> +{
> +    /* g_get_monotonic_time() is best-effort so we can use the wall clock as a
> +     * fallback.
> +     */
> +
> +    GTimeVal time;
> +    g_get_current_time(&time);
> +
> +    return time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
> +}
> +/* work around distro backports of this interface */
> +#define g_get_monotonic_time() qemu_g_get_monotonic_time()
> +#endif
> +
> +#if defined(_WIN32) && !GLIB_CHECK_VERSION(2, 50, 0)
> +/*
> + * g_poll has a problem on Windows when using
> + * timeouts < 10ms, so use wrapper.
> + */
> +#define g_poll(fds, nfds, timeout) g_poll_fixed(fds, nfds, timeout)
> +gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
> +#endif
> +
> +#if !GLIB_CHECK_VERSION(2, 30, 0)
> +/* Not a 100% compatible implementation, but good enough for most
> + * cases. Placeholders are only supported at the end of the
> + * template. */
> +static inline gchar *qemu_g_dir_make_tmp(gchar const *tmpl, GError **error)
> +{
> +    gchar *path = g_build_filename(g_get_tmp_dir(), tmpl ?: ".XXXXXX", NULL);
> +
> +    if (mkdtemp(path) != NULL) {
> +        return path;
> +    }
> +    /* Error occurred, clean up. */
> +    g_set_error(error, G_FILE_ERROR, g_file_error_from_errno(errno),
> +                "mkdtemp() failed");
> +    g_free(path);
> +    return NULL;
> +}
> +#define g_dir_make_tmp(tmpl, error) qemu_g_dir_make_tmp(tmpl, error)
> +#endif /* glib 2.30 */
> +
> +#if !GLIB_CHECK_VERSION(2, 31, 0)
> +/* before glib-2.31, GMutex and GCond was dynamic-only (there was a separate
> + * GStaticMutex, but it didn't work with condition variables).
> + *
> + * Our implementation uses GOnce to fake a static implementation that does
> + * not require separate initialization.
> + * We need to rename the types to avoid passing our CompatGMutex/CompatGCond
> + * by mistake to a function that expects GMutex/GCond.  However, for ease
> + * of use we keep the GLib function names.  GLib uses macros for the
> + * implementation, we use inline functions instead and undefine the macros.
> + */
> +
> +typedef struct CompatGMutex {
> +    GOnce once;
> +} CompatGMutex;
> +
> +typedef struct CompatGCond {
> +    GOnce once;
> +} CompatGCond;
> +
> +static inline gpointer do_g_mutex_new(gpointer unused)
> +{
> +    return (gpointer) g_mutex_new();
> +}
> +
> +static inline void g_mutex_init(CompatGMutex *mutex)
> +{
> +    mutex->once = (GOnce) G_ONCE_INIT;
> +}
> +
> +static inline void g_mutex_clear(CompatGMutex *mutex)
> +{
> +    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> +    if (mutex->once.retval) {
> +        g_mutex_free((GMutex *) mutex->once.retval);
> +    }
> +    mutex->once = (GOnce) G_ONCE_INIT;
> +}
> +
> +static inline void (g_mutex_lock)(CompatGMutex *mutex)
> +{
> +    g_once(&mutex->once, do_g_mutex_new, NULL);
> +    g_mutex_lock((GMutex *) mutex->once.retval);
> +}
> +#undef g_mutex_lock
> +
> +static inline gboolean (g_mutex_trylock)(CompatGMutex *mutex)
> +{
> +    g_once(&mutex->once, do_g_mutex_new, NULL);
> +    return g_mutex_trylock((GMutex *) mutex->once.retval);
> +}
> +#undef g_mutex_trylock
> +
> +
> +static inline void (g_mutex_unlock)(CompatGMutex *mutex)
> +{
> +    g_mutex_unlock((GMutex *) mutex->once.retval);
> +}
> +#undef g_mutex_unlock
> +
> +static inline gpointer do_g_cond_new(gpointer unused)
> +{
> +    return (gpointer) g_cond_new();
> +}
> +
> +static inline void g_cond_init(CompatGCond *cond)
> +{
> +    cond->once = (GOnce) G_ONCE_INIT;
> +}
> +
> +static inline void g_cond_clear(CompatGCond *cond)
> +{
> +    g_assert(cond->once.status != G_ONCE_STATUS_PROGRESS);
> +    if (cond->once.retval) {
> +        g_cond_free((GCond *) cond->once.retval);
> +    }
> +    cond->once = (GOnce) G_ONCE_INIT;
> +}
> +
> +static inline void (g_cond_wait)(CompatGCond *cond, CompatGMutex *mutex)
> +{
> +    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> +    g_once(&cond->once, do_g_cond_new, NULL);
> +    g_cond_wait((GCond *) cond->once.retval, (GMutex *) mutex->once.retval);
> +}
> +#undef g_cond_wait
> +
> +static inline void (g_cond_broadcast)(CompatGCond *cond)
> +{
> +    g_once(&cond->once, do_g_cond_new, NULL);
> +    g_cond_broadcast((GCond *) cond->once.retval);
> +}
> +#undef g_cond_broadcast
> +
> +static inline void (g_cond_signal)(CompatGCond *cond)
> +{
> +    g_once(&cond->once, do_g_cond_new, NULL);
> +    g_cond_signal((GCond *) cond->once.retval);
> +}
> +#undef g_cond_signal
> +
> +static inline gboolean (g_cond_timed_wait)(CompatGCond *cond,
> +                                           CompatGMutex *mutex,
> +                                           GTimeVal *time)
> +{
> +    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> +    g_once(&cond->once, do_g_cond_new, NULL);
> +    return g_cond_timed_wait((GCond *) cond->once.retval,
> +                             (GMutex *) mutex->once.retval, time);
> +}
> +#undef g_cond_timed_wait
> +
> +/* This is not a macro, because it didn't exist until 2.32.  */
> +static inline gboolean g_cond_wait_until(CompatGCond *cond, CompatGMutex *mutex,
> +                                         gint64 end_time)
> +{
> +    GTimeVal time;
> +
> +    /* Convert from monotonic to CLOCK_REALTIME.  */
> +    end_time -= g_get_monotonic_time();
> +    g_get_current_time(&time);
> +    end_time += time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
> +
> +    time.tv_sec = end_time / G_TIME_SPAN_SECOND;
> +    time.tv_usec = end_time % G_TIME_SPAN_SECOND;
> +    return g_cond_timed_wait(cond, mutex, &time);
> +}
> +
> +/* before 2.31 there was no g_thread_new() */
> +static inline GThread *g_thread_new(const char *name,
> +                                    GThreadFunc func, gpointer data)
> +{
> +    GThread *thread = g_thread_create(func, data, TRUE, NULL);
> +    if (!thread) {
> +        g_error("creating thread");
> +    }
> +    return thread;
> +}
> +#else
> +#define CompatGMutex GMutex
> +#define CompatGCond GCond
> +#endif /* glib 2.31 */
> +
> +#if !GLIB_CHECK_VERSION(2, 32, 0)
> +/* Beware, function returns gboolean since 2.39.2, see GLib commit 9101915 */
> +static inline void g_hash_table_add(GHashTable *hash_table, gpointer key)
> +{
> +    g_hash_table_replace(hash_table, key, key);
> +}
> +#endif
> +
> +#ifndef g_assert_true
> +#define g_assert_true(expr)                                                    \
> +    do {                                                                       \
> +        if (G_LIKELY(expr)) {                                                  \
> +        } else {                                                               \
> +            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> +                                "'" #expr "' should be TRUE");                 \
> +        }                                                                      \
> +    } while (0)
> +#endif
> +
> +#ifndef g_assert_false
> +#define g_assert_false(expr)                                                   \
> +    do {                                                                       \
> +        if (G_LIKELY(!(expr))) {                                               \
> +        } else {                                                               \
> +            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> +                                "'" #expr "' should be FALSE");                \
> +        }                                                                      \
> +    } while (0)
> +#endif
> +
> +#ifndef g_assert_null
> +#define g_assert_null(expr)                                                    \
> +    do {                                                                       \
> +        if (G_LIKELY((expr) == NULL)) {                                        \
> +        } else {                                                               \
> +            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> +                                "'" #expr "' should be NULL");                 \
> +        }                                                                      \
> +    } while (0)
> +#endif
> +
> +#ifndef g_assert_nonnull
> +#define g_assert_nonnull(expr)                                                 \
> +    do {                                                                       \
> +        if (G_LIKELY((expr) != NULL)) {                                        \
> +        } else {                                                               \
> +            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> +                                "'" #expr "' should not be NULL");             \
> +        }                                                                      \
> +    } while (0)
> +#endif
> +
> +#ifndef g_assert_cmpmem
> +#define g_assert_cmpmem(m1, l1, m2, l2)                                        \
> +    do {                                                                       \
> +        gconstpointer __m1 = m1, __m2 = m2;                                    \
> +        int __l1 = l1, __l2 = l2;                                              \
> +        if (__l1 != __l2) {                                                    \
> +            g_assertion_message_cmpnum(                                        \
> +                G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,                   \
> +                #l1 " (len(" #m1 ")) == " #l2 " (len(" #m2 "))", __l1, "==",   \
> +                __l2, 'i');                                                    \
> +        } else if (memcmp(__m1, __m2, __l1) != 0) {                            \
> +            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> +                                "assertion failed (" #m1 " == " #m2 ")");      \
> +        }                                                                      \
> +    } while (0)
> +#endif
> +
> +#if !GLIB_CHECK_VERSION(2, 28, 0)
> +static inline void g_list_free_full(GList *list, GDestroyNotify free_func)
> +{
> +    GList *l;
> +
> +    for (l = list; l; l = l->next) {
> +        free_func(l->data);
> +    }
> +
> +    g_list_free(list);
> +}
> +
> +static inline void g_slist_free_full(GSList *list, GDestroyNotify free_func)
> +{
> +    GSList *l;
> +
> +    for (l = list; l; l = l->next) {
> +        free_func(l->data);
> +    }
> +
> +    g_slist_free(list);
> +}
> +#endif
> +
> +#if !GLIB_CHECK_VERSION(2, 26, 0)
> +static inline void g_source_set_name(GSource *source, const char *name)
> +{
> +    /* This is just a debugging aid, so leaving it a no-op */
> +}
> +static inline void g_source_set_name_by_id(guint tag, const char *name)
> +{
> +    /* This is just a debugging aid, so leaving it a no-op */
> +}
> +#endif
> +
> +#if !GLIB_CHECK_VERSION(2, 36, 0)
> +/* Always fail.  This will not include error_report output in the test log,
> + * sending it instead to stderr.
> + */
> +#define g_test_initialized() (0)
> +#endif
> +#if !GLIB_CHECK_VERSION(2, 38, 0)
> +#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
> +#error schizophrenic detection of glib subprocess testing
> +#endif
> +#define g_test_subprocess() (0)
> +#endif
> +
> +
> +#if !GLIB_CHECK_VERSION(2, 34, 0)
> +static inline void
> +g_test_add_data_func_full(const char *path,
> +                          gpointer data,
> +                          gpointer fn,
> +                          gpointer data_free_func)
> +{
> +#if GLIB_CHECK_VERSION(2, 26, 0)
> +    /* back-compat casts, remove this once we can require new-enough glib */
> +    g_test_add_vtable(path, 0, data, NULL,
> +                      (GTestFixtureFunc)fn, (GTestFixtureFunc) data_free_func);
> +#else
> +    /* back-compat casts, remove this once we can require new-enough glib */
> +    g_test_add_vtable(path, 0, data, NULL,
> +                      (void (*)(void)) fn, (void (*)(void)) data_free_func);
> +#endif
> +}
> +#endif
> +
> +
> +#endif
> diff --git a/include/glib/glib-helper.h b/include/glib/glib-helper.h
> new file mode 100644
> index 0000000..db740fb
> --- /dev/null
> +++ b/include/glib/glib-helper.h
> @@ -0,0 +1,30 @@
> +/*
> + * Helpers for GLIB
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef QEMU_GLIB_HELPER_H
> +#define QEMU_GLIB_HELPER_H
> +
> +
> +#include "glib/glib-compat.h"
> +
> +#define GPOINTER_TO_UINT64(a) ((guint64) (a))
> +
> +/*
> + * return 1 in case of a > b, -1 otherwise and 0 if equeal
> + */
> +gint g_int_cmp64(gconstpointer a, gconstpointer b,
> +        gpointer __attribute__((unused)) user_data);
> +
> +/*
> + * return 1 in case of a > b, -1 otherwise and 0 if equeal
> + */
> +int g_int_cmp(gconstpointer a, gconstpointer b,
> +        gpointer __attribute__((unused)) user_data);
> +
> +#endif /* QEMU_GLIB_HELPER_H */
> +
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 122ff06..36f8a89 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -104,7 +104,7 @@ extern int daemon(int, int);
>  #include "sysemu/os-posix.h"
>  #endif
>
> -#include "glib-compat.h"
> +#include "glib/glib-compat.h"
>  #include "qemu/typedefs.h"
>
>  #ifndef O_LARGEFILE
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 10a3bb3..7cea6bc 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -35,7 +35,7 @@
>  #include "elf.h"
>  #include "exec/log.h"
>  #include "trace/control.h"
> -#include "glib-compat.h"
> +#include "glib/glib-compat.h"
>
>  char *exec_path;
>
> diff --git a/scripts/clean-includes b/scripts/clean-includes
> index dd938da..b32b928 100755
> --- a/scripts/clean-includes
> +++ b/scripts/clean-includes
> @@ -123,7 +123,7 @@ for f in "$@"; do
>        ;;
>      *include/qemu/osdep.h | \
>      *include/qemu/compiler.h | \
> -    *include/glib-compat.h | \
> +    *include/glib/glib-compat.h | \
>      *include/sysemu/os-posix.h | \
>      *include/sysemu/os-win32.h | \
>      *include/standard-headers/ )
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index c6205eb..0080712 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -43,3 +43,4 @@ util-obj-y += qdist.o
>  util-obj-y += qht.o
>  util-obj-y += range.o
>  util-obj-y += systemd.o
> +util-obj-y += glib-helper.o
> diff --git a/util/glib-helper.c b/util/glib-helper.c
> new file mode 100644
> index 0000000..2557009
> --- /dev/null
> +++ b/util/glib-helper.c
> @@ -0,0 +1,29 @@
> +/*
> + * Implementation for GLIB helpers
> + * this file is intented to commulate and later reuse
> + * additional glib functions
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> +
> + */
> +
> +#include "glib/glib-helper.h"
> +
> +gint g_int_cmp64(gconstpointer a, gconstpointer b,
> +        gpointer __attribute__((unused)) user_data)
> +{
> +    guint64 ua = GPOINTER_TO_UINT64(a);
> +    guint64 ub = GPOINTER_TO_UINT64(b);
> +    return (ua > ub) - (ua < ub);
> +}
> +
> +/*
> + * return 1 in case of a > b, -1 otherwise and 0 if equeal
> + */
> +gint g_int_cmp(gconstpointer a, gconstpointer b,
> +        gpointer __attribute__((unused)) user_data)
> +{
> +    return g_int_cmp64(a, b, user_data);
> +}
> +
>

Re: [Qemu-devel] [PATCH 2/6] util: introduce glib-helper.c
Posted by Alexey 8 years, 9 months ago
Hi Philippe, 

On Fri, Apr 14, 2017 at 01:05:52PM -0300, Philippe Mathieu-Daudé wrote:
> Hi Alexey,
> 
> On 04/14/2017 10:17 AM, Alexey Perevalov wrote:
> >There is a lack of g_int_cmp which compares pointers value in glib,
> >xen_disk.c introduced its own, so the same function now requires
> >in migration.c. So logically to move it into common place.
> >Futher: maybe extend glib.
> >
> >Also this commit moves existing glib-compat.h into util/glib
> >folder for consolidation purpose.
> 
> Can you do this in two commits? First one moving files only, second
> move the function?
> 
Ok
> I'm not sure naming it "g_int_cmp()" won't clash with future
> _extended_ glib, what do you think about naming it
> "qemu_g_int_cmp()"?
> 
Why not, if it will have better maintainability.

> >Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> >---
> > hw/block/xen_disk.c        |  10 +-
> > include/glib-compat.h      | 352 ---------------------------------------------
> > include/glib/glib-compat.h | 352 +++++++++++++++++++++++++++++++++++++++++++++
> > include/glib/glib-helper.h |  30 ++++
> > include/qemu/osdep.h       |   2 +-
> > linux-user/main.c          |   2 +-
> > scripts/clean-includes     |   2 +-
> > util/Makefile.objs         |   1 +
> > util/glib-helper.c         |  29 ++++
> > 9 files changed, 417 insertions(+), 363 deletions(-)
> > delete mode 100644 include/glib-compat.h
> > create mode 100644 include/glib/glib-compat.h
> > create mode 100644 include/glib/glib-helper.h
> > create mode 100644 util/glib-helper.c
> >
> >diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> >index 456a2d5..36f6396 100644
> >--- a/hw/block/xen_disk.c
> >+++ b/hw/block/xen_disk.c
> >@@ -20,6 +20,7 @@
> >  */
> >
> > #include "qemu/osdep.h"
> >+#include "glib/glib-helper.h"
> > #include <sys/ioctl.h>
> > #include <sys/uio.h>
> >
> >@@ -154,13 +155,6 @@ static void ioreq_reset(struct ioreq *ioreq)
> >     qemu_iovec_reset(&ioreq->v);
> > }
> >
> >-static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
> >-{
> >-    uint ua = GPOINTER_TO_UINT(a);
> >-    uint ub = GPOINTER_TO_UINT(b);
> >-    return (ua > ub) - (ua < ub);
> >-}
> >-
> > static void destroy_grant(gpointer pgnt)
> > {
> >     PersistentGrant *grant = pgnt;
> >@@ -1191,7 +1185,7 @@ static int blk_connect(struct XenDevice *xendev)
> >     if (blkdev->feature_persistent) {
> >         /* Init persistent grants */
> >         blkdev->max_grants = max_requests * BLKIF_MAX_SEGMENTS_PER_REQUEST;
> >-        blkdev->persistent_gnts = g_tree_new_full((GCompareDataFunc)int_cmp,
> >+        blkdev->persistent_gnts = g_tree_new_full((GCompareDataFunc)g_int_cmp,
> >                                              NULL, NULL,
> >                                              batch_maps ?
> >                                              (GDestroyNotify)g_free :
> >diff --git a/include/glib-compat.h b/include/glib-compat.h
> >deleted file mode 100644
> >index 863c8cf..0000000
> >--- a/include/glib-compat.h
> >+++ /dev/null
> >@@ -1,352 +0,0 @@
> >-/*
> >- * GLIB Compatibility Functions
> >- *
> >- * Copyright IBM, Corp. 2013
> >- *
> >- * Authors:
> >- *  Anthony Liguori   <aliguori@us.ibm.com>
> >- *  Michael Tokarev   <mjt@tls.msk.ru>
> >- *  Paolo Bonzini     <pbonzini@redhat.com>
> >- *
> >- * This work is licensed under the terms of the GNU GPL, version 2 or later.
> >- * See the COPYING file in the top-level directory.
> >- *
> >- */
> >-
> >-#ifndef QEMU_GLIB_COMPAT_H
> >-#define QEMU_GLIB_COMPAT_H
> >-
> >-#include <glib.h>
> >-
> >-/* GLIB version compatibility flags */
> >-#if !GLIB_CHECK_VERSION(2, 26, 0)
> >-#define G_TIME_SPAN_SECOND              (G_GINT64_CONSTANT(1000000))
> >-#endif
> >-
> >-#if !GLIB_CHECK_VERSION(2, 28, 0)
> >-static inline gint64 qemu_g_get_monotonic_time(void)
> >-{
> >-    /* g_get_monotonic_time() is best-effort so we can use the wall clock as a
> >-     * fallback.
> >-     */
> >-
> >-    GTimeVal time;
> >-    g_get_current_time(&time);
> >-
> >-    return time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
> >-}
> >-/* work around distro backports of this interface */
> >-#define g_get_monotonic_time() qemu_g_get_monotonic_time()
> >-#endif
> >-
> >-#if defined(_WIN32) && !GLIB_CHECK_VERSION(2, 50, 0)
> >-/*
> >- * g_poll has a problem on Windows when using
> >- * timeouts < 10ms, so use wrapper.
> >- */
> >-#define g_poll(fds, nfds, timeout) g_poll_fixed(fds, nfds, timeout)
> >-gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
> >-#endif
> >-
> >-#if !GLIB_CHECK_VERSION(2, 30, 0)
> >-/* Not a 100% compatible implementation, but good enough for most
> >- * cases. Placeholders are only supported at the end of the
> >- * template. */
> >-static inline gchar *qemu_g_dir_make_tmp(gchar const *tmpl, GError **error)
> >-{
> >-    gchar *path = g_build_filename(g_get_tmp_dir(), tmpl ?: ".XXXXXX", NULL);
> >-
> >-    if (mkdtemp(path) != NULL) {
> >-        return path;
> >-    }
> >-    /* Error occurred, clean up. */
> >-    g_set_error(error, G_FILE_ERROR, g_file_error_from_errno(errno),
> >-                "mkdtemp() failed");
> >-    g_free(path);
> >-    return NULL;
> >-}
> >-#define g_dir_make_tmp(tmpl, error) qemu_g_dir_make_tmp(tmpl, error)
> >-#endif /* glib 2.30 */
> >-
> >-#if !GLIB_CHECK_VERSION(2, 31, 0)
> >-/* before glib-2.31, GMutex and GCond was dynamic-only (there was a separate
> >- * GStaticMutex, but it didn't work with condition variables).
> >- *
> >- * Our implementation uses GOnce to fake a static implementation that does
> >- * not require separate initialization.
> >- * We need to rename the types to avoid passing our CompatGMutex/CompatGCond
> >- * by mistake to a function that expects GMutex/GCond.  However, for ease
> >- * of use we keep the GLib function names.  GLib uses macros for the
> >- * implementation, we use inline functions instead and undefine the macros.
> >- */
> >-
> >-typedef struct CompatGMutex {
> >-    GOnce once;
> >-} CompatGMutex;
> >-
> >-typedef struct CompatGCond {
> >-    GOnce once;
> >-} CompatGCond;
> >-
> >-static inline gpointer do_g_mutex_new(gpointer unused)
> >-{
> >-    return (gpointer) g_mutex_new();
> >-}
> >-
> >-static inline void g_mutex_init(CompatGMutex *mutex)
> >-{
> >-    mutex->once = (GOnce) G_ONCE_INIT;
> >-}
> >-
> >-static inline void g_mutex_clear(CompatGMutex *mutex)
> >-{
> >-    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> >-    if (mutex->once.retval) {
> >-        g_mutex_free((GMutex *) mutex->once.retval);
> >-    }
> >-    mutex->once = (GOnce) G_ONCE_INIT;
> >-}
> >-
> >-static inline void (g_mutex_lock)(CompatGMutex *mutex)
> >-{
> >-    g_once(&mutex->once, do_g_mutex_new, NULL);
> >-    g_mutex_lock((GMutex *) mutex->once.retval);
> >-}
> >-#undef g_mutex_lock
> >-
> >-static inline gboolean (g_mutex_trylock)(CompatGMutex *mutex)
> >-{
> >-    g_once(&mutex->once, do_g_mutex_new, NULL);
> >-    return g_mutex_trylock((GMutex *) mutex->once.retval);
> >-}
> >-#undef g_mutex_trylock
> >-
> >-
> >-static inline void (g_mutex_unlock)(CompatGMutex *mutex)
> >-{
> >-    g_mutex_unlock((GMutex *) mutex->once.retval);
> >-}
> >-#undef g_mutex_unlock
> >-
> >-static inline gpointer do_g_cond_new(gpointer unused)
> >-{
> >-    return (gpointer) g_cond_new();
> >-}
> >-
> >-static inline void g_cond_init(CompatGCond *cond)
> >-{
> >-    cond->once = (GOnce) G_ONCE_INIT;
> >-}
> >-
> >-static inline void g_cond_clear(CompatGCond *cond)
> >-{
> >-    g_assert(cond->once.status != G_ONCE_STATUS_PROGRESS);
> >-    if (cond->once.retval) {
> >-        g_cond_free((GCond *) cond->once.retval);
> >-    }
> >-    cond->once = (GOnce) G_ONCE_INIT;
> >-}
> >-
> >-static inline void (g_cond_wait)(CompatGCond *cond, CompatGMutex *mutex)
> >-{
> >-    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> >-    g_once(&cond->once, do_g_cond_new, NULL);
> >-    g_cond_wait((GCond *) cond->once.retval, (GMutex *) mutex->once.retval);
> >-}
> >-#undef g_cond_wait
> >-
> >-static inline void (g_cond_broadcast)(CompatGCond *cond)
> >-{
> >-    g_once(&cond->once, do_g_cond_new, NULL);
> >-    g_cond_broadcast((GCond *) cond->once.retval);
> >-}
> >-#undef g_cond_broadcast
> >-
> >-static inline void (g_cond_signal)(CompatGCond *cond)
> >-{
> >-    g_once(&cond->once, do_g_cond_new, NULL);
> >-    g_cond_signal((GCond *) cond->once.retval);
> >-}
> >-#undef g_cond_signal
> >-
> >-static inline gboolean (g_cond_timed_wait)(CompatGCond *cond,
> >-                                           CompatGMutex *mutex,
> >-                                           GTimeVal *time)
> >-{
> >-    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> >-    g_once(&cond->once, do_g_cond_new, NULL);
> >-    return g_cond_timed_wait((GCond *) cond->once.retval,
> >-                             (GMutex *) mutex->once.retval, time);
> >-}
> >-#undef g_cond_timed_wait
> >-
> >-/* This is not a macro, because it didn't exist until 2.32.  */
> >-static inline gboolean g_cond_wait_until(CompatGCond *cond, CompatGMutex *mutex,
> >-                                         gint64 end_time)
> >-{
> >-    GTimeVal time;
> >-
> >-    /* Convert from monotonic to CLOCK_REALTIME.  */
> >-    end_time -= g_get_monotonic_time();
> >-    g_get_current_time(&time);
> >-    end_time += time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
> >-
> >-    time.tv_sec = end_time / G_TIME_SPAN_SECOND;
> >-    time.tv_usec = end_time % G_TIME_SPAN_SECOND;
> >-    return g_cond_timed_wait(cond, mutex, &time);
> >-}
> >-
> >-/* before 2.31 there was no g_thread_new() */
> >-static inline GThread *g_thread_new(const char *name,
> >-                                    GThreadFunc func, gpointer data)
> >-{
> >-    GThread *thread = g_thread_create(func, data, TRUE, NULL);
> >-    if (!thread) {
> >-        g_error("creating thread");
> >-    }
> >-    return thread;
> >-}
> >-#else
> >-#define CompatGMutex GMutex
> >-#define CompatGCond GCond
> >-#endif /* glib 2.31 */
> >-
> >-#if !GLIB_CHECK_VERSION(2, 32, 0)
> >-/* Beware, function returns gboolean since 2.39.2, see GLib commit 9101915 */
> >-static inline void g_hash_table_add(GHashTable *hash_table, gpointer key)
> >-{
> >-    g_hash_table_replace(hash_table, key, key);
> >-}
> >-#endif
> >-
> >-#ifndef g_assert_true
> >-#define g_assert_true(expr)                                                    \
> >-    do {                                                                       \
> >-        if (G_LIKELY(expr)) {                                                  \
> >-        } else {                                                               \
> >-            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> >-                                "'" #expr "' should be TRUE");                 \
> >-        }                                                                      \
> >-    } while (0)
> >-#endif
> >-
> >-#ifndef g_assert_false
> >-#define g_assert_false(expr)                                                   \
> >-    do {                                                                       \
> >-        if (G_LIKELY(!(expr))) {                                               \
> >-        } else {                                                               \
> >-            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> >-                                "'" #expr "' should be FALSE");                \
> >-        }                                                                      \
> >-    } while (0)
> >-#endif
> >-
> >-#ifndef g_assert_null
> >-#define g_assert_null(expr)                                                    \
> >-    do {                                                                       \
> >-        if (G_LIKELY((expr) == NULL)) {                                        \
> >-        } else {                                                               \
> >-            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> >-                                "'" #expr "' should be NULL");                 \
> >-        }                                                                      \
> >-    } while (0)
> >-#endif
> >-
> >-#ifndef g_assert_nonnull
> >-#define g_assert_nonnull(expr)                                                 \
> >-    do {                                                                       \
> >-        if (G_LIKELY((expr) != NULL)) {                                        \
> >-        } else {                                                               \
> >-            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> >-                                "'" #expr "' should not be NULL");             \
> >-        }                                                                      \
> >-    } while (0)
> >-#endif
> >-
> >-#ifndef g_assert_cmpmem
> >-#define g_assert_cmpmem(m1, l1, m2, l2)                                        \
> >-    do {                                                                       \
> >-        gconstpointer __m1 = m1, __m2 = m2;                                    \
> >-        int __l1 = l1, __l2 = l2;                                              \
> >-        if (__l1 != __l2) {                                                    \
> >-            g_assertion_message_cmpnum(                                        \
> >-                G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,                   \
> >-                #l1 " (len(" #m1 ")) == " #l2 " (len(" #m2 "))", __l1, "==",   \
> >-                __l2, 'i');                                                    \
> >-        } else if (memcmp(__m1, __m2, __l1) != 0) {                            \
> >-            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> >-                                "assertion failed (" #m1 " == " #m2 ")");      \
> >-        }                                                                      \
> >-    } while (0)
> >-#endif
> >-
> >-#if !GLIB_CHECK_VERSION(2, 28, 0)
> >-static inline void g_list_free_full(GList *list, GDestroyNotify free_func)
> >-{
> >-    GList *l;
> >-
> >-    for (l = list; l; l = l->next) {
> >-        free_func(l->data);
> >-    }
> >-
> >-    g_list_free(list);
> >-}
> >-
> >-static inline void g_slist_free_full(GSList *list, GDestroyNotify free_func)
> >-{
> >-    GSList *l;
> >-
> >-    for (l = list; l; l = l->next) {
> >-        free_func(l->data);
> >-    }
> >-
> >-    g_slist_free(list);
> >-}
> >-#endif
> >-
> >-#if !GLIB_CHECK_VERSION(2, 26, 0)
> >-static inline void g_source_set_name(GSource *source, const char *name)
> >-{
> >-    /* This is just a debugging aid, so leaving it a no-op */
> >-}
> >-static inline void g_source_set_name_by_id(guint tag, const char *name)
> >-{
> >-    /* This is just a debugging aid, so leaving it a no-op */
> >-}
> >-#endif
> >-
> >-#if !GLIB_CHECK_VERSION(2, 36, 0)
> >-/* Always fail.  This will not include error_report output in the test log,
> >- * sending it instead to stderr.
> >- */
> >-#define g_test_initialized() (0)
> >-#endif
> >-#if !GLIB_CHECK_VERSION(2, 38, 0)
> >-#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
> >-#error schizophrenic detection of glib subprocess testing
> >-#endif
> >-#define g_test_subprocess() (0)
> >-#endif
> >-
> >-
> >-#if !GLIB_CHECK_VERSION(2, 34, 0)
> >-static inline void
> >-g_test_add_data_func_full(const char *path,
> >-                          gpointer data,
> >-                          gpointer fn,
> >-                          gpointer data_free_func)
> >-{
> >-#if GLIB_CHECK_VERSION(2, 26, 0)
> >-    /* back-compat casts, remove this once we can require new-enough glib */
> >-    g_test_add_vtable(path, 0, data, NULL,
> >-                      (GTestFixtureFunc)fn, (GTestFixtureFunc) data_free_func);
> >-#else
> >-    /* back-compat casts, remove this once we can require new-enough glib */
> >-    g_test_add_vtable(path, 0, data, NULL,
> >-                      (void (*)(void)) fn, (void (*)(void)) data_free_func);
> >-#endif
> >-}
> >-#endif
> >-
> >-
> >-#endif
> >diff --git a/include/glib/glib-compat.h b/include/glib/glib-compat.h
> >new file mode 100644
> >index 0000000..863c8cf
> >--- /dev/null
> >+++ b/include/glib/glib-compat.h
> >@@ -0,0 +1,352 @@
> >+/*
> >+ * GLIB Compatibility Functions
> >+ *
> >+ * Copyright IBM, Corp. 2013
> >+ *
> >+ * Authors:
> >+ *  Anthony Liguori   <aliguori@us.ibm.com>
> >+ *  Michael Tokarev   <mjt@tls.msk.ru>
> >+ *  Paolo Bonzini     <pbonzini@redhat.com>
> >+ *
> >+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
> >+ * See the COPYING file in the top-level directory.
> >+ *
> >+ */
> >+
> >+#ifndef QEMU_GLIB_COMPAT_H
> >+#define QEMU_GLIB_COMPAT_H
> >+
> >+#include <glib.h>
> >+
> >+/* GLIB version compatibility flags */
> >+#if !GLIB_CHECK_VERSION(2, 26, 0)
> >+#define G_TIME_SPAN_SECOND              (G_GINT64_CONSTANT(1000000))
> >+#endif
> >+
> >+#if !GLIB_CHECK_VERSION(2, 28, 0)
> >+static inline gint64 qemu_g_get_monotonic_time(void)
> >+{
> >+    /* g_get_monotonic_time() is best-effort so we can use the wall clock as a
> >+     * fallback.
> >+     */
> >+
> >+    GTimeVal time;
> >+    g_get_current_time(&time);
> >+
> >+    return time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
> >+}
> >+/* work around distro backports of this interface */
> >+#define g_get_monotonic_time() qemu_g_get_monotonic_time()
> >+#endif
> >+
> >+#if defined(_WIN32) && !GLIB_CHECK_VERSION(2, 50, 0)
> >+/*
> >+ * g_poll has a problem on Windows when using
> >+ * timeouts < 10ms, so use wrapper.
> >+ */
> >+#define g_poll(fds, nfds, timeout) g_poll_fixed(fds, nfds, timeout)
> >+gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
> >+#endif
> >+
> >+#if !GLIB_CHECK_VERSION(2, 30, 0)
> >+/* Not a 100% compatible implementation, but good enough for most
> >+ * cases. Placeholders are only supported at the end of the
> >+ * template. */
> >+static inline gchar *qemu_g_dir_make_tmp(gchar const *tmpl, GError **error)
> >+{
> >+    gchar *path = g_build_filename(g_get_tmp_dir(), tmpl ?: ".XXXXXX", NULL);
> >+
> >+    if (mkdtemp(path) != NULL) {
> >+        return path;
> >+    }
> >+    /* Error occurred, clean up. */
> >+    g_set_error(error, G_FILE_ERROR, g_file_error_from_errno(errno),
> >+                "mkdtemp() failed");
> >+    g_free(path);
> >+    return NULL;
> >+}
> >+#define g_dir_make_tmp(tmpl, error) qemu_g_dir_make_tmp(tmpl, error)
> >+#endif /* glib 2.30 */
> >+
> >+#if !GLIB_CHECK_VERSION(2, 31, 0)
> >+/* before glib-2.31, GMutex and GCond was dynamic-only (there was a separate
> >+ * GStaticMutex, but it didn't work with condition variables).
> >+ *
> >+ * Our implementation uses GOnce to fake a static implementation that does
> >+ * not require separate initialization.
> >+ * We need to rename the types to avoid passing our CompatGMutex/CompatGCond
> >+ * by mistake to a function that expects GMutex/GCond.  However, for ease
> >+ * of use we keep the GLib function names.  GLib uses macros for the
> >+ * implementation, we use inline functions instead and undefine the macros.
> >+ */
> >+
> >+typedef struct CompatGMutex {
> >+    GOnce once;
> >+} CompatGMutex;
> >+
> >+typedef struct CompatGCond {
> >+    GOnce once;
> >+} CompatGCond;
> >+
> >+static inline gpointer do_g_mutex_new(gpointer unused)
> >+{
> >+    return (gpointer) g_mutex_new();
> >+}
> >+
> >+static inline void g_mutex_init(CompatGMutex *mutex)
> >+{
> >+    mutex->once = (GOnce) G_ONCE_INIT;
> >+}
> >+
> >+static inline void g_mutex_clear(CompatGMutex *mutex)
> >+{
> >+    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> >+    if (mutex->once.retval) {
> >+        g_mutex_free((GMutex *) mutex->once.retval);
> >+    }
> >+    mutex->once = (GOnce) G_ONCE_INIT;
> >+}
> >+
> >+static inline void (g_mutex_lock)(CompatGMutex *mutex)
> >+{
> >+    g_once(&mutex->once, do_g_mutex_new, NULL);
> >+    g_mutex_lock((GMutex *) mutex->once.retval);
> >+}
> >+#undef g_mutex_lock
> >+
> >+static inline gboolean (g_mutex_trylock)(CompatGMutex *mutex)
> >+{
> >+    g_once(&mutex->once, do_g_mutex_new, NULL);
> >+    return g_mutex_trylock((GMutex *) mutex->once.retval);
> >+}
> >+#undef g_mutex_trylock
> >+
> >+
> >+static inline void (g_mutex_unlock)(CompatGMutex *mutex)
> >+{
> >+    g_mutex_unlock((GMutex *) mutex->once.retval);
> >+}
> >+#undef g_mutex_unlock
> >+
> >+static inline gpointer do_g_cond_new(gpointer unused)
> >+{
> >+    return (gpointer) g_cond_new();
> >+}
> >+
> >+static inline void g_cond_init(CompatGCond *cond)
> >+{
> >+    cond->once = (GOnce) G_ONCE_INIT;
> >+}
> >+
> >+static inline void g_cond_clear(CompatGCond *cond)
> >+{
> >+    g_assert(cond->once.status != G_ONCE_STATUS_PROGRESS);
> >+    if (cond->once.retval) {
> >+        g_cond_free((GCond *) cond->once.retval);
> >+    }
> >+    cond->once = (GOnce) G_ONCE_INIT;
> >+}
> >+
> >+static inline void (g_cond_wait)(CompatGCond *cond, CompatGMutex *mutex)
> >+{
> >+    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> >+    g_once(&cond->once, do_g_cond_new, NULL);
> >+    g_cond_wait((GCond *) cond->once.retval, (GMutex *) mutex->once.retval);
> >+}
> >+#undef g_cond_wait
> >+
> >+static inline void (g_cond_broadcast)(CompatGCond *cond)
> >+{
> >+    g_once(&cond->once, do_g_cond_new, NULL);
> >+    g_cond_broadcast((GCond *) cond->once.retval);
> >+}
> >+#undef g_cond_broadcast
> >+
> >+static inline void (g_cond_signal)(CompatGCond *cond)
> >+{
> >+    g_once(&cond->once, do_g_cond_new, NULL);
> >+    g_cond_signal((GCond *) cond->once.retval);
> >+}
> >+#undef g_cond_signal
> >+
> >+static inline gboolean (g_cond_timed_wait)(CompatGCond *cond,
> >+                                           CompatGMutex *mutex,
> >+                                           GTimeVal *time)
> >+{
> >+    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> >+    g_once(&cond->once, do_g_cond_new, NULL);
> >+    return g_cond_timed_wait((GCond *) cond->once.retval,
> >+                             (GMutex *) mutex->once.retval, time);
> >+}
> >+#undef g_cond_timed_wait
> >+
> >+/* This is not a macro, because it didn't exist until 2.32.  */
> >+static inline gboolean g_cond_wait_until(CompatGCond *cond, CompatGMutex *mutex,
> >+                                         gint64 end_time)
> >+{
> >+    GTimeVal time;
> >+
> >+    /* Convert from monotonic to CLOCK_REALTIME.  */
> >+    end_time -= g_get_monotonic_time();
> >+    g_get_current_time(&time);
> >+    end_time += time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
> >+
> >+    time.tv_sec = end_time / G_TIME_SPAN_SECOND;
> >+    time.tv_usec = end_time % G_TIME_SPAN_SECOND;
> >+    return g_cond_timed_wait(cond, mutex, &time);
> >+}
> >+
> >+/* before 2.31 there was no g_thread_new() */
> >+static inline GThread *g_thread_new(const char *name,
> >+                                    GThreadFunc func, gpointer data)
> >+{
> >+    GThread *thread = g_thread_create(func, data, TRUE, NULL);
> >+    if (!thread) {
> >+        g_error("creating thread");
> >+    }
> >+    return thread;
> >+}
> >+#else
> >+#define CompatGMutex GMutex
> >+#define CompatGCond GCond
> >+#endif /* glib 2.31 */
> >+
> >+#if !GLIB_CHECK_VERSION(2, 32, 0)
> >+/* Beware, function returns gboolean since 2.39.2, see GLib commit 9101915 */
> >+static inline void g_hash_table_add(GHashTable *hash_table, gpointer key)
> >+{
> >+    g_hash_table_replace(hash_table, key, key);
> >+}
> >+#endif
> >+
> >+#ifndef g_assert_true
> >+#define g_assert_true(expr)                                                    \
> >+    do {                                                                       \
> >+        if (G_LIKELY(expr)) {                                                  \
> >+        } else {                                                               \
> >+            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> >+                                "'" #expr "' should be TRUE");                 \
> >+        }                                                                      \
> >+    } while (0)
> >+#endif
> >+
> >+#ifndef g_assert_false
> >+#define g_assert_false(expr)                                                   \
> >+    do {                                                                       \
> >+        if (G_LIKELY(!(expr))) {                                               \
> >+        } else {                                                               \
> >+            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> >+                                "'" #expr "' should be FALSE");                \
> >+        }                                                                      \
> >+    } while (0)
> >+#endif
> >+
> >+#ifndef g_assert_null
> >+#define g_assert_null(expr)                                                    \
> >+    do {                                                                       \
> >+        if (G_LIKELY((expr) == NULL)) {                                        \
> >+        } else {                                                               \
> >+            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> >+                                "'" #expr "' should be NULL");                 \
> >+        }                                                                      \
> >+    } while (0)
> >+#endif
> >+
> >+#ifndef g_assert_nonnull
> >+#define g_assert_nonnull(expr)                                                 \
> >+    do {                                                                       \
> >+        if (G_LIKELY((expr) != NULL)) {                                        \
> >+        } else {                                                               \
> >+            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> >+                                "'" #expr "' should not be NULL");             \
> >+        }                                                                      \
> >+    } while (0)
> >+#endif
> >+
> >+#ifndef g_assert_cmpmem
> >+#define g_assert_cmpmem(m1, l1, m2, l2)                                        \
> >+    do {                                                                       \
> >+        gconstpointer __m1 = m1, __m2 = m2;                                    \
> >+        int __l1 = l1, __l2 = l2;                                              \
> >+        if (__l1 != __l2) {                                                    \
> >+            g_assertion_message_cmpnum(                                        \
> >+                G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,                   \
> >+                #l1 " (len(" #m1 ")) == " #l2 " (len(" #m2 "))", __l1, "==",   \
> >+                __l2, 'i');                                                    \
> >+        } else if (memcmp(__m1, __m2, __l1) != 0) {                            \
> >+            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> >+                                "assertion failed (" #m1 " == " #m2 ")");      \
> >+        }                                                                      \
> >+    } while (0)
> >+#endif
> >+
> >+#if !GLIB_CHECK_VERSION(2, 28, 0)
> >+static inline void g_list_free_full(GList *list, GDestroyNotify free_func)
> >+{
> >+    GList *l;
> >+
> >+    for (l = list; l; l = l->next) {
> >+        free_func(l->data);
> >+    }
> >+
> >+    g_list_free(list);
> >+}
> >+
> >+static inline void g_slist_free_full(GSList *list, GDestroyNotify free_func)
> >+{
> >+    GSList *l;
> >+
> >+    for (l = list; l; l = l->next) {
> >+        free_func(l->data);
> >+    }
> >+
> >+    g_slist_free(list);
> >+}
> >+#endif
> >+
> >+#if !GLIB_CHECK_VERSION(2, 26, 0)
> >+static inline void g_source_set_name(GSource *source, const char *name)
> >+{
> >+    /* This is just a debugging aid, so leaving it a no-op */
> >+}
> >+static inline void g_source_set_name_by_id(guint tag, const char *name)
> >+{
> >+    /* This is just a debugging aid, so leaving it a no-op */
> >+}
> >+#endif
> >+
> >+#if !GLIB_CHECK_VERSION(2, 36, 0)
> >+/* Always fail.  This will not include error_report output in the test log,
> >+ * sending it instead to stderr.
> >+ */
> >+#define g_test_initialized() (0)
> >+#endif
> >+#if !GLIB_CHECK_VERSION(2, 38, 0)
> >+#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
> >+#error schizophrenic detection of glib subprocess testing
> >+#endif
> >+#define g_test_subprocess() (0)
> >+#endif
> >+
> >+
> >+#if !GLIB_CHECK_VERSION(2, 34, 0)
> >+static inline void
> >+g_test_add_data_func_full(const char *path,
> >+                          gpointer data,
> >+                          gpointer fn,
> >+                          gpointer data_free_func)
> >+{
> >+#if GLIB_CHECK_VERSION(2, 26, 0)
> >+    /* back-compat casts, remove this once we can require new-enough glib */
> >+    g_test_add_vtable(path, 0, data, NULL,
> >+                      (GTestFixtureFunc)fn, (GTestFixtureFunc) data_free_func);
> >+#else
> >+    /* back-compat casts, remove this once we can require new-enough glib */
> >+    g_test_add_vtable(path, 0, data, NULL,
> >+                      (void (*)(void)) fn, (void (*)(void)) data_free_func);
> >+#endif
> >+}
> >+#endif
> >+
> >+
> >+#endif
> >diff --git a/include/glib/glib-helper.h b/include/glib/glib-helper.h
> >new file mode 100644
> >index 0000000..db740fb
> >--- /dev/null
> >+++ b/include/glib/glib-helper.h
> >@@ -0,0 +1,30 @@
> >+/*
> >+ * Helpers for GLIB
> >+ *
> >+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
> >+ * See the COPYING file in the top-level directory.
> >+ *
> >+ */
> >+
> >+#ifndef QEMU_GLIB_HELPER_H
> >+#define QEMU_GLIB_HELPER_H
> >+
> >+
> >+#include "glib/glib-compat.h"
> >+
> >+#define GPOINTER_TO_UINT64(a) ((guint64) (a))
> >+
> >+/*
> >+ * return 1 in case of a > b, -1 otherwise and 0 if equeal
> >+ */
> >+gint g_int_cmp64(gconstpointer a, gconstpointer b,
> >+        gpointer __attribute__((unused)) user_data);
> >+
> >+/*
> >+ * return 1 in case of a > b, -1 otherwise and 0 if equeal
> >+ */
> >+int g_int_cmp(gconstpointer a, gconstpointer b,
> >+        gpointer __attribute__((unused)) user_data);
> >+
> >+#endif /* QEMU_GLIB_HELPER_H */
> >+
> >diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> >index 122ff06..36f8a89 100644
> >--- a/include/qemu/osdep.h
> >+++ b/include/qemu/osdep.h
> >@@ -104,7 +104,7 @@ extern int daemon(int, int);
> > #include "sysemu/os-posix.h"
> > #endif
> >
> >-#include "glib-compat.h"
> >+#include "glib/glib-compat.h"
> > #include "qemu/typedefs.h"
> >
> > #ifndef O_LARGEFILE
> >diff --git a/linux-user/main.c b/linux-user/main.c
> >index 10a3bb3..7cea6bc 100644
> >--- a/linux-user/main.c
> >+++ b/linux-user/main.c
> >@@ -35,7 +35,7 @@
> > #include "elf.h"
> > #include "exec/log.h"
> > #include "trace/control.h"
> >-#include "glib-compat.h"
> >+#include "glib/glib-compat.h"
> >
> > char *exec_path;
> >
> >diff --git a/scripts/clean-includes b/scripts/clean-includes
> >index dd938da..b32b928 100755
> >--- a/scripts/clean-includes
> >+++ b/scripts/clean-includes
> >@@ -123,7 +123,7 @@ for f in "$@"; do
> >       ;;
> >     *include/qemu/osdep.h | \
> >     *include/qemu/compiler.h | \
> >-    *include/glib-compat.h | \
> >+    *include/glib/glib-compat.h | \
> >     *include/sysemu/os-posix.h | \
> >     *include/sysemu/os-win32.h | \
> >     *include/standard-headers/ )
> >diff --git a/util/Makefile.objs b/util/Makefile.objs
> >index c6205eb..0080712 100644
> >--- a/util/Makefile.objs
> >+++ b/util/Makefile.objs
> >@@ -43,3 +43,4 @@ util-obj-y += qdist.o
> > util-obj-y += qht.o
> > util-obj-y += range.o
> > util-obj-y += systemd.o
> >+util-obj-y += glib-helper.o
> >diff --git a/util/glib-helper.c b/util/glib-helper.c
> >new file mode 100644
> >index 0000000..2557009
> >--- /dev/null
> >+++ b/util/glib-helper.c
> >@@ -0,0 +1,29 @@
> >+/*
> >+ * Implementation for GLIB helpers
> >+ * this file is intented to commulate and later reuse
> >+ * additional glib functions
> >+ *
> >+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
> >+ * See the COPYING file in the top-level directory.
> >+
> >+ */
> >+
> >+#include "glib/glib-helper.h"
> >+
> >+gint g_int_cmp64(gconstpointer a, gconstpointer b,
> >+        gpointer __attribute__((unused)) user_data)
> >+{
> >+    guint64 ua = GPOINTER_TO_UINT64(a);
> >+    guint64 ub = GPOINTER_TO_UINT64(b);
> >+    return (ua > ub) - (ua < ub);
> >+}
> >+
> >+/*
> >+ * return 1 in case of a > b, -1 otherwise and 0 if equeal
> >+ */
> >+gint g_int_cmp(gconstpointer a, gconstpointer b,
> >+        gpointer __attribute__((unused)) user_data)
> >+{
> >+    return g_int_cmp64(a, b, user_data);
> >+}
> >+
> >
> 

-- 

BR
Alexey


Re: [Qemu-devel] [PATCH 2/6] util: introduce glib-helper.c
Posted by Dr. David Alan Gilbert 8 years, 9 months ago
* Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
> Hi Alexey,
> 
> On 04/14/2017 10:17 AM, Alexey Perevalov wrote:
> > There is a lack of g_int_cmp which compares pointers value in glib,
> > xen_disk.c introduced its own, so the same function now requires
> > in migration.c. So logically to move it into common place.
> > Futher: maybe extend glib.
> > 
> > Also this commit moves existing glib-compat.h into util/glib
> > folder for consolidation purpose.
> 
> Can you do this in two commits? First one moving files only, second move the
> function?

Yes, if you're lucky and do it with git mv  then perhaps git  will generate us a nice
small commit with a move in it rather than a vast delete/add.

> I'm not sure naming it "g_int_cmp()" won't clash with future _extended_
> glib, what do you think about naming it "qemu_g_int_cmp()"?

Also agreed.

Dave

> 
> > Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> > ---
> >  hw/block/xen_disk.c        |  10 +-
> >  include/glib-compat.h      | 352 ---------------------------------------------
> >  include/glib/glib-compat.h | 352 +++++++++++++++++++++++++++++++++++++++++++++
> >  include/glib/glib-helper.h |  30 ++++
> >  include/qemu/osdep.h       |   2 +-
> >  linux-user/main.c          |   2 +-
> >  scripts/clean-includes     |   2 +-
> >  util/Makefile.objs         |   1 +
> >  util/glib-helper.c         |  29 ++++
> >  9 files changed, 417 insertions(+), 363 deletions(-)
> >  delete mode 100644 include/glib-compat.h
> >  create mode 100644 include/glib/glib-compat.h
> >  create mode 100644 include/glib/glib-helper.h
> >  create mode 100644 util/glib-helper.c
> > 
> > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> > index 456a2d5..36f6396 100644
> > --- a/hw/block/xen_disk.c
> > +++ b/hw/block/xen_disk.c
> > @@ -20,6 +20,7 @@
> >   */
> > 
> >  #include "qemu/osdep.h"
> > +#include "glib/glib-helper.h"
> >  #include <sys/ioctl.h>
> >  #include <sys/uio.h>
> > 
> > @@ -154,13 +155,6 @@ static void ioreq_reset(struct ioreq *ioreq)
> >      qemu_iovec_reset(&ioreq->v);
> >  }
> > 
> > -static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
> > -{
> > -    uint ua = GPOINTER_TO_UINT(a);
> > -    uint ub = GPOINTER_TO_UINT(b);
> > -    return (ua > ub) - (ua < ub);
> > -}
> > -
> >  static void destroy_grant(gpointer pgnt)
> >  {
> >      PersistentGrant *grant = pgnt;
> > @@ -1191,7 +1185,7 @@ static int blk_connect(struct XenDevice *xendev)
> >      if (blkdev->feature_persistent) {
> >          /* Init persistent grants */
> >          blkdev->max_grants = max_requests * BLKIF_MAX_SEGMENTS_PER_REQUEST;
> > -        blkdev->persistent_gnts = g_tree_new_full((GCompareDataFunc)int_cmp,
> > +        blkdev->persistent_gnts = g_tree_new_full((GCompareDataFunc)g_int_cmp,
> >                                               NULL, NULL,
> >                                               batch_maps ?
> >                                               (GDestroyNotify)g_free :
> > diff --git a/include/glib-compat.h b/include/glib-compat.h
> > deleted file mode 100644
> > index 863c8cf..0000000
> > --- a/include/glib-compat.h
> > +++ /dev/null
> > @@ -1,352 +0,0 @@
> > -/*
> > - * GLIB Compatibility Functions
> > - *
> > - * Copyright IBM, Corp. 2013
> > - *
> > - * Authors:
> > - *  Anthony Liguori   <aliguori@us.ibm.com>
> > - *  Michael Tokarev   <mjt@tls.msk.ru>
> > - *  Paolo Bonzini     <pbonzini@redhat.com>
> > - *
> > - * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > - * See the COPYING file in the top-level directory.
> > - *
> > - */
> > -
> > -#ifndef QEMU_GLIB_COMPAT_H
> > -#define QEMU_GLIB_COMPAT_H
> > -
> > -#include <glib.h>
> > -
> > -/* GLIB version compatibility flags */
> > -#if !GLIB_CHECK_VERSION(2, 26, 0)
> > -#define G_TIME_SPAN_SECOND              (G_GINT64_CONSTANT(1000000))
> > -#endif
> > -
> > -#if !GLIB_CHECK_VERSION(2, 28, 0)
> > -static inline gint64 qemu_g_get_monotonic_time(void)
> > -{
> > -    /* g_get_monotonic_time() is best-effort so we can use the wall clock as a
> > -     * fallback.
> > -     */
> > -
> > -    GTimeVal time;
> > -    g_get_current_time(&time);
> > -
> > -    return time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
> > -}
> > -/* work around distro backports of this interface */
> > -#define g_get_monotonic_time() qemu_g_get_monotonic_time()
> > -#endif
> > -
> > -#if defined(_WIN32) && !GLIB_CHECK_VERSION(2, 50, 0)
> > -/*
> > - * g_poll has a problem on Windows when using
> > - * timeouts < 10ms, so use wrapper.
> > - */
> > -#define g_poll(fds, nfds, timeout) g_poll_fixed(fds, nfds, timeout)
> > -gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
> > -#endif
> > -
> > -#if !GLIB_CHECK_VERSION(2, 30, 0)
> > -/* Not a 100% compatible implementation, but good enough for most
> > - * cases. Placeholders are only supported at the end of the
> > - * template. */
> > -static inline gchar *qemu_g_dir_make_tmp(gchar const *tmpl, GError **error)
> > -{
> > -    gchar *path = g_build_filename(g_get_tmp_dir(), tmpl ?: ".XXXXXX", NULL);
> > -
> > -    if (mkdtemp(path) != NULL) {
> > -        return path;
> > -    }
> > -    /* Error occurred, clean up. */
> > -    g_set_error(error, G_FILE_ERROR, g_file_error_from_errno(errno),
> > -                "mkdtemp() failed");
> > -    g_free(path);
> > -    return NULL;
> > -}
> > -#define g_dir_make_tmp(tmpl, error) qemu_g_dir_make_tmp(tmpl, error)
> > -#endif /* glib 2.30 */
> > -
> > -#if !GLIB_CHECK_VERSION(2, 31, 0)
> > -/* before glib-2.31, GMutex and GCond was dynamic-only (there was a separate
> > - * GStaticMutex, but it didn't work with condition variables).
> > - *
> > - * Our implementation uses GOnce to fake a static implementation that does
> > - * not require separate initialization.
> > - * We need to rename the types to avoid passing our CompatGMutex/CompatGCond
> > - * by mistake to a function that expects GMutex/GCond.  However, for ease
> > - * of use we keep the GLib function names.  GLib uses macros for the
> > - * implementation, we use inline functions instead and undefine the macros.
> > - */
> > -
> > -typedef struct CompatGMutex {
> > -    GOnce once;
> > -} CompatGMutex;
> > -
> > -typedef struct CompatGCond {
> > -    GOnce once;
> > -} CompatGCond;
> > -
> > -static inline gpointer do_g_mutex_new(gpointer unused)
> > -{
> > -    return (gpointer) g_mutex_new();
> > -}
> > -
> > -static inline void g_mutex_init(CompatGMutex *mutex)
> > -{
> > -    mutex->once = (GOnce) G_ONCE_INIT;
> > -}
> > -
> > -static inline void g_mutex_clear(CompatGMutex *mutex)
> > -{
> > -    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> > -    if (mutex->once.retval) {
> > -        g_mutex_free((GMutex *) mutex->once.retval);
> > -    }
> > -    mutex->once = (GOnce) G_ONCE_INIT;
> > -}
> > -
> > -static inline void (g_mutex_lock)(CompatGMutex *mutex)
> > -{
> > -    g_once(&mutex->once, do_g_mutex_new, NULL);
> > -    g_mutex_lock((GMutex *) mutex->once.retval);
> > -}
> > -#undef g_mutex_lock
> > -
> > -static inline gboolean (g_mutex_trylock)(CompatGMutex *mutex)
> > -{
> > -    g_once(&mutex->once, do_g_mutex_new, NULL);
> > -    return g_mutex_trylock((GMutex *) mutex->once.retval);
> > -}
> > -#undef g_mutex_trylock
> > -
> > -
> > -static inline void (g_mutex_unlock)(CompatGMutex *mutex)
> > -{
> > -    g_mutex_unlock((GMutex *) mutex->once.retval);
> > -}
> > -#undef g_mutex_unlock
> > -
> > -static inline gpointer do_g_cond_new(gpointer unused)
> > -{
> > -    return (gpointer) g_cond_new();
> > -}
> > -
> > -static inline void g_cond_init(CompatGCond *cond)
> > -{
> > -    cond->once = (GOnce) G_ONCE_INIT;
> > -}
> > -
> > -static inline void g_cond_clear(CompatGCond *cond)
> > -{
> > -    g_assert(cond->once.status != G_ONCE_STATUS_PROGRESS);
> > -    if (cond->once.retval) {
> > -        g_cond_free((GCond *) cond->once.retval);
> > -    }
> > -    cond->once = (GOnce) G_ONCE_INIT;
> > -}
> > -
> > -static inline void (g_cond_wait)(CompatGCond *cond, CompatGMutex *mutex)
> > -{
> > -    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> > -    g_once(&cond->once, do_g_cond_new, NULL);
> > -    g_cond_wait((GCond *) cond->once.retval, (GMutex *) mutex->once.retval);
> > -}
> > -#undef g_cond_wait
> > -
> > -static inline void (g_cond_broadcast)(CompatGCond *cond)
> > -{
> > -    g_once(&cond->once, do_g_cond_new, NULL);
> > -    g_cond_broadcast((GCond *) cond->once.retval);
> > -}
> > -#undef g_cond_broadcast
> > -
> > -static inline void (g_cond_signal)(CompatGCond *cond)
> > -{
> > -    g_once(&cond->once, do_g_cond_new, NULL);
> > -    g_cond_signal((GCond *) cond->once.retval);
> > -}
> > -#undef g_cond_signal
> > -
> > -static inline gboolean (g_cond_timed_wait)(CompatGCond *cond,
> > -                                           CompatGMutex *mutex,
> > -                                           GTimeVal *time)
> > -{
> > -    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> > -    g_once(&cond->once, do_g_cond_new, NULL);
> > -    return g_cond_timed_wait((GCond *) cond->once.retval,
> > -                             (GMutex *) mutex->once.retval, time);
> > -}
> > -#undef g_cond_timed_wait
> > -
> > -/* This is not a macro, because it didn't exist until 2.32.  */
> > -static inline gboolean g_cond_wait_until(CompatGCond *cond, CompatGMutex *mutex,
> > -                                         gint64 end_time)
> > -{
> > -    GTimeVal time;
> > -
> > -    /* Convert from monotonic to CLOCK_REALTIME.  */
> > -    end_time -= g_get_monotonic_time();
> > -    g_get_current_time(&time);
> > -    end_time += time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
> > -
> > -    time.tv_sec = end_time / G_TIME_SPAN_SECOND;
> > -    time.tv_usec = end_time % G_TIME_SPAN_SECOND;
> > -    return g_cond_timed_wait(cond, mutex, &time);
> > -}
> > -
> > -/* before 2.31 there was no g_thread_new() */
> > -static inline GThread *g_thread_new(const char *name,
> > -                                    GThreadFunc func, gpointer data)
> > -{
> > -    GThread *thread = g_thread_create(func, data, TRUE, NULL);
> > -    if (!thread) {
> > -        g_error("creating thread");
> > -    }
> > -    return thread;
> > -}
> > -#else
> > -#define CompatGMutex GMutex
> > -#define CompatGCond GCond
> > -#endif /* glib 2.31 */
> > -
> > -#if !GLIB_CHECK_VERSION(2, 32, 0)
> > -/* Beware, function returns gboolean since 2.39.2, see GLib commit 9101915 */
> > -static inline void g_hash_table_add(GHashTable *hash_table, gpointer key)
> > -{
> > -    g_hash_table_replace(hash_table, key, key);
> > -}
> > -#endif
> > -
> > -#ifndef g_assert_true
> > -#define g_assert_true(expr)                                                    \
> > -    do {                                                                       \
> > -        if (G_LIKELY(expr)) {                                                  \
> > -        } else {                                                               \
> > -            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> > -                                "'" #expr "' should be TRUE");                 \
> > -        }                                                                      \
> > -    } while (0)
> > -#endif
> > -
> > -#ifndef g_assert_false
> > -#define g_assert_false(expr)                                                   \
> > -    do {                                                                       \
> > -        if (G_LIKELY(!(expr))) {                                               \
> > -        } else {                                                               \
> > -            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> > -                                "'" #expr "' should be FALSE");                \
> > -        }                                                                      \
> > -    } while (0)
> > -#endif
> > -
> > -#ifndef g_assert_null
> > -#define g_assert_null(expr)                                                    \
> > -    do {                                                                       \
> > -        if (G_LIKELY((expr) == NULL)) {                                        \
> > -        } else {                                                               \
> > -            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> > -                                "'" #expr "' should be NULL");                 \
> > -        }                                                                      \
> > -    } while (0)
> > -#endif
> > -
> > -#ifndef g_assert_nonnull
> > -#define g_assert_nonnull(expr)                                                 \
> > -    do {                                                                       \
> > -        if (G_LIKELY((expr) != NULL)) {                                        \
> > -        } else {                                                               \
> > -            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> > -                                "'" #expr "' should not be NULL");             \
> > -        }                                                                      \
> > -    } while (0)
> > -#endif
> > -
> > -#ifndef g_assert_cmpmem
> > -#define g_assert_cmpmem(m1, l1, m2, l2)                                        \
> > -    do {                                                                       \
> > -        gconstpointer __m1 = m1, __m2 = m2;                                    \
> > -        int __l1 = l1, __l2 = l2;                                              \
> > -        if (__l1 != __l2) {                                                    \
> > -            g_assertion_message_cmpnum(                                        \
> > -                G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,                   \
> > -                #l1 " (len(" #m1 ")) == " #l2 " (len(" #m2 "))", __l1, "==",   \
> > -                __l2, 'i');                                                    \
> > -        } else if (memcmp(__m1, __m2, __l1) != 0) {                            \
> > -            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> > -                                "assertion failed (" #m1 " == " #m2 ")");      \
> > -        }                                                                      \
> > -    } while (0)
> > -#endif
> > -
> > -#if !GLIB_CHECK_VERSION(2, 28, 0)
> > -static inline void g_list_free_full(GList *list, GDestroyNotify free_func)
> > -{
> > -    GList *l;
> > -
> > -    for (l = list; l; l = l->next) {
> > -        free_func(l->data);
> > -    }
> > -
> > -    g_list_free(list);
> > -}
> > -
> > -static inline void g_slist_free_full(GSList *list, GDestroyNotify free_func)
> > -{
> > -    GSList *l;
> > -
> > -    for (l = list; l; l = l->next) {
> > -        free_func(l->data);
> > -    }
> > -
> > -    g_slist_free(list);
> > -}
> > -#endif
> > -
> > -#if !GLIB_CHECK_VERSION(2, 26, 0)
> > -static inline void g_source_set_name(GSource *source, const char *name)
> > -{
> > -    /* This is just a debugging aid, so leaving it a no-op */
> > -}
> > -static inline void g_source_set_name_by_id(guint tag, const char *name)
> > -{
> > -    /* This is just a debugging aid, so leaving it a no-op */
> > -}
> > -#endif
> > -
> > -#if !GLIB_CHECK_VERSION(2, 36, 0)
> > -/* Always fail.  This will not include error_report output in the test log,
> > - * sending it instead to stderr.
> > - */
> > -#define g_test_initialized() (0)
> > -#endif
> > -#if !GLIB_CHECK_VERSION(2, 38, 0)
> > -#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
> > -#error schizophrenic detection of glib subprocess testing
> > -#endif
> > -#define g_test_subprocess() (0)
> > -#endif
> > -
> > -
> > -#if !GLIB_CHECK_VERSION(2, 34, 0)
> > -static inline void
> > -g_test_add_data_func_full(const char *path,
> > -                          gpointer data,
> > -                          gpointer fn,
> > -                          gpointer data_free_func)
> > -{
> > -#if GLIB_CHECK_VERSION(2, 26, 0)
> > -    /* back-compat casts, remove this once we can require new-enough glib */
> > -    g_test_add_vtable(path, 0, data, NULL,
> > -                      (GTestFixtureFunc)fn, (GTestFixtureFunc) data_free_func);
> > -#else
> > -    /* back-compat casts, remove this once we can require new-enough glib */
> > -    g_test_add_vtable(path, 0, data, NULL,
> > -                      (void (*)(void)) fn, (void (*)(void)) data_free_func);
> > -#endif
> > -}
> > -#endif
> > -
> > -
> > -#endif
> > diff --git a/include/glib/glib-compat.h b/include/glib/glib-compat.h
> > new file mode 100644
> > index 0000000..863c8cf
> > --- /dev/null
> > +++ b/include/glib/glib-compat.h
> > @@ -0,0 +1,352 @@
> > +/*
> > + * GLIB Compatibility Functions
> > + *
> > + * Copyright IBM, Corp. 2013
> > + *
> > + * Authors:
> > + *  Anthony Liguori   <aliguori@us.ibm.com>
> > + *  Michael Tokarev   <mjt@tls.msk.ru>
> > + *  Paolo Bonzini     <pbonzini@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#ifndef QEMU_GLIB_COMPAT_H
> > +#define QEMU_GLIB_COMPAT_H
> > +
> > +#include <glib.h>
> > +
> > +/* GLIB version compatibility flags */
> > +#if !GLIB_CHECK_VERSION(2, 26, 0)
> > +#define G_TIME_SPAN_SECOND              (G_GINT64_CONSTANT(1000000))
> > +#endif
> > +
> > +#if !GLIB_CHECK_VERSION(2, 28, 0)
> > +static inline gint64 qemu_g_get_monotonic_time(void)
> > +{
> > +    /* g_get_monotonic_time() is best-effort so we can use the wall clock as a
> > +     * fallback.
> > +     */
> > +
> > +    GTimeVal time;
> > +    g_get_current_time(&time);
> > +
> > +    return time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
> > +}
> > +/* work around distro backports of this interface */
> > +#define g_get_monotonic_time() qemu_g_get_monotonic_time()
> > +#endif
> > +
> > +#if defined(_WIN32) && !GLIB_CHECK_VERSION(2, 50, 0)
> > +/*
> > + * g_poll has a problem on Windows when using
> > + * timeouts < 10ms, so use wrapper.
> > + */
> > +#define g_poll(fds, nfds, timeout) g_poll_fixed(fds, nfds, timeout)
> > +gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
> > +#endif
> > +
> > +#if !GLIB_CHECK_VERSION(2, 30, 0)
> > +/* Not a 100% compatible implementation, but good enough for most
> > + * cases. Placeholders are only supported at the end of the
> > + * template. */
> > +static inline gchar *qemu_g_dir_make_tmp(gchar const *tmpl, GError **error)
> > +{
> > +    gchar *path = g_build_filename(g_get_tmp_dir(), tmpl ?: ".XXXXXX", NULL);
> > +
> > +    if (mkdtemp(path) != NULL) {
> > +        return path;
> > +    }
> > +    /* Error occurred, clean up. */
> > +    g_set_error(error, G_FILE_ERROR, g_file_error_from_errno(errno),
> > +                "mkdtemp() failed");
> > +    g_free(path);
> > +    return NULL;
> > +}
> > +#define g_dir_make_tmp(tmpl, error) qemu_g_dir_make_tmp(tmpl, error)
> > +#endif /* glib 2.30 */
> > +
> > +#if !GLIB_CHECK_VERSION(2, 31, 0)
> > +/* before glib-2.31, GMutex and GCond was dynamic-only (there was a separate
> > + * GStaticMutex, but it didn't work with condition variables).
> > + *
> > + * Our implementation uses GOnce to fake a static implementation that does
> > + * not require separate initialization.
> > + * We need to rename the types to avoid passing our CompatGMutex/CompatGCond
> > + * by mistake to a function that expects GMutex/GCond.  However, for ease
> > + * of use we keep the GLib function names.  GLib uses macros for the
> > + * implementation, we use inline functions instead and undefine the macros.
> > + */
> > +
> > +typedef struct CompatGMutex {
> > +    GOnce once;
> > +} CompatGMutex;
> > +
> > +typedef struct CompatGCond {
> > +    GOnce once;
> > +} CompatGCond;
> > +
> > +static inline gpointer do_g_mutex_new(gpointer unused)
> > +{
> > +    return (gpointer) g_mutex_new();
> > +}
> > +
> > +static inline void g_mutex_init(CompatGMutex *mutex)
> > +{
> > +    mutex->once = (GOnce) G_ONCE_INIT;
> > +}
> > +
> > +static inline void g_mutex_clear(CompatGMutex *mutex)
> > +{
> > +    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> > +    if (mutex->once.retval) {
> > +        g_mutex_free((GMutex *) mutex->once.retval);
> > +    }
> > +    mutex->once = (GOnce) G_ONCE_INIT;
> > +}
> > +
> > +static inline void (g_mutex_lock)(CompatGMutex *mutex)
> > +{
> > +    g_once(&mutex->once, do_g_mutex_new, NULL);
> > +    g_mutex_lock((GMutex *) mutex->once.retval);
> > +}
> > +#undef g_mutex_lock
> > +
> > +static inline gboolean (g_mutex_trylock)(CompatGMutex *mutex)
> > +{
> > +    g_once(&mutex->once, do_g_mutex_new, NULL);
> > +    return g_mutex_trylock((GMutex *) mutex->once.retval);
> > +}
> > +#undef g_mutex_trylock
> > +
> > +
> > +static inline void (g_mutex_unlock)(CompatGMutex *mutex)
> > +{
> > +    g_mutex_unlock((GMutex *) mutex->once.retval);
> > +}
> > +#undef g_mutex_unlock
> > +
> > +static inline gpointer do_g_cond_new(gpointer unused)
> > +{
> > +    return (gpointer) g_cond_new();
> > +}
> > +
> > +static inline void g_cond_init(CompatGCond *cond)
> > +{
> > +    cond->once = (GOnce) G_ONCE_INIT;
> > +}
> > +
> > +static inline void g_cond_clear(CompatGCond *cond)
> > +{
> > +    g_assert(cond->once.status != G_ONCE_STATUS_PROGRESS);
> > +    if (cond->once.retval) {
> > +        g_cond_free((GCond *) cond->once.retval);
> > +    }
> > +    cond->once = (GOnce) G_ONCE_INIT;
> > +}
> > +
> > +static inline void (g_cond_wait)(CompatGCond *cond, CompatGMutex *mutex)
> > +{
> > +    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> > +    g_once(&cond->once, do_g_cond_new, NULL);
> > +    g_cond_wait((GCond *) cond->once.retval, (GMutex *) mutex->once.retval);
> > +}
> > +#undef g_cond_wait
> > +
> > +static inline void (g_cond_broadcast)(CompatGCond *cond)
> > +{
> > +    g_once(&cond->once, do_g_cond_new, NULL);
> > +    g_cond_broadcast((GCond *) cond->once.retval);
> > +}
> > +#undef g_cond_broadcast
> > +
> > +static inline void (g_cond_signal)(CompatGCond *cond)
> > +{
> > +    g_once(&cond->once, do_g_cond_new, NULL);
> > +    g_cond_signal((GCond *) cond->once.retval);
> > +}
> > +#undef g_cond_signal
> > +
> > +static inline gboolean (g_cond_timed_wait)(CompatGCond *cond,
> > +                                           CompatGMutex *mutex,
> > +                                           GTimeVal *time)
> > +{
> > +    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
> > +    g_once(&cond->once, do_g_cond_new, NULL);
> > +    return g_cond_timed_wait((GCond *) cond->once.retval,
> > +                             (GMutex *) mutex->once.retval, time);
> > +}
> > +#undef g_cond_timed_wait
> > +
> > +/* This is not a macro, because it didn't exist until 2.32.  */
> > +static inline gboolean g_cond_wait_until(CompatGCond *cond, CompatGMutex *mutex,
> > +                                         gint64 end_time)
> > +{
> > +    GTimeVal time;
> > +
> > +    /* Convert from monotonic to CLOCK_REALTIME.  */
> > +    end_time -= g_get_monotonic_time();
> > +    g_get_current_time(&time);
> > +    end_time += time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
> > +
> > +    time.tv_sec = end_time / G_TIME_SPAN_SECOND;
> > +    time.tv_usec = end_time % G_TIME_SPAN_SECOND;
> > +    return g_cond_timed_wait(cond, mutex, &time);
> > +}
> > +
> > +/* before 2.31 there was no g_thread_new() */
> > +static inline GThread *g_thread_new(const char *name,
> > +                                    GThreadFunc func, gpointer data)
> > +{
> > +    GThread *thread = g_thread_create(func, data, TRUE, NULL);
> > +    if (!thread) {
> > +        g_error("creating thread");
> > +    }
> > +    return thread;
> > +}
> > +#else
> > +#define CompatGMutex GMutex
> > +#define CompatGCond GCond
> > +#endif /* glib 2.31 */
> > +
> > +#if !GLIB_CHECK_VERSION(2, 32, 0)
> > +/* Beware, function returns gboolean since 2.39.2, see GLib commit 9101915 */
> > +static inline void g_hash_table_add(GHashTable *hash_table, gpointer key)
> > +{
> > +    g_hash_table_replace(hash_table, key, key);
> > +}
> > +#endif
> > +
> > +#ifndef g_assert_true
> > +#define g_assert_true(expr)                                                    \
> > +    do {                                                                       \
> > +        if (G_LIKELY(expr)) {                                                  \
> > +        } else {                                                               \
> > +            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> > +                                "'" #expr "' should be TRUE");                 \
> > +        }                                                                      \
> > +    } while (0)
> > +#endif
> > +
> > +#ifndef g_assert_false
> > +#define g_assert_false(expr)                                                   \
> > +    do {                                                                       \
> > +        if (G_LIKELY(!(expr))) {                                               \
> > +        } else {                                                               \
> > +            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> > +                                "'" #expr "' should be FALSE");                \
> > +        }                                                                      \
> > +    } while (0)
> > +#endif
> > +
> > +#ifndef g_assert_null
> > +#define g_assert_null(expr)                                                    \
> > +    do {                                                                       \
> > +        if (G_LIKELY((expr) == NULL)) {                                        \
> > +        } else {                                                               \
> > +            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> > +                                "'" #expr "' should be NULL");                 \
> > +        }                                                                      \
> > +    } while (0)
> > +#endif
> > +
> > +#ifndef g_assert_nonnull
> > +#define g_assert_nonnull(expr)                                                 \
> > +    do {                                                                       \
> > +        if (G_LIKELY((expr) != NULL)) {                                        \
> > +        } else {                                                               \
> > +            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> > +                                "'" #expr "' should not be NULL");             \
> > +        }                                                                      \
> > +    } while (0)
> > +#endif
> > +
> > +#ifndef g_assert_cmpmem
> > +#define g_assert_cmpmem(m1, l1, m2, l2)                                        \
> > +    do {                                                                       \
> > +        gconstpointer __m1 = m1, __m2 = m2;                                    \
> > +        int __l1 = l1, __l2 = l2;                                              \
> > +        if (__l1 != __l2) {                                                    \
> > +            g_assertion_message_cmpnum(                                        \
> > +                G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,                   \
> > +                #l1 " (len(" #m1 ")) == " #l2 " (len(" #m2 "))", __l1, "==",   \
> > +                __l2, 'i');                                                    \
> > +        } else if (memcmp(__m1, __m2, __l1) != 0) {                            \
> > +            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
> > +                                "assertion failed (" #m1 " == " #m2 ")");      \
> > +        }                                                                      \
> > +    } while (0)
> > +#endif
> > +
> > +#if !GLIB_CHECK_VERSION(2, 28, 0)
> > +static inline void g_list_free_full(GList *list, GDestroyNotify free_func)
> > +{
> > +    GList *l;
> > +
> > +    for (l = list; l; l = l->next) {
> > +        free_func(l->data);
> > +    }
> > +
> > +    g_list_free(list);
> > +}
> > +
> > +static inline void g_slist_free_full(GSList *list, GDestroyNotify free_func)
> > +{
> > +    GSList *l;
> > +
> > +    for (l = list; l; l = l->next) {
> > +        free_func(l->data);
> > +    }
> > +
> > +    g_slist_free(list);
> > +}
> > +#endif
> > +
> > +#if !GLIB_CHECK_VERSION(2, 26, 0)
> > +static inline void g_source_set_name(GSource *source, const char *name)
> > +{
> > +    /* This is just a debugging aid, so leaving it a no-op */
> > +}
> > +static inline void g_source_set_name_by_id(guint tag, const char *name)
> > +{
> > +    /* This is just a debugging aid, so leaving it a no-op */
> > +}
> > +#endif
> > +
> > +#if !GLIB_CHECK_VERSION(2, 36, 0)
> > +/* Always fail.  This will not include error_report output in the test log,
> > + * sending it instead to stderr.
> > + */
> > +#define g_test_initialized() (0)
> > +#endif
> > +#if !GLIB_CHECK_VERSION(2, 38, 0)
> > +#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
> > +#error schizophrenic detection of glib subprocess testing
> > +#endif
> > +#define g_test_subprocess() (0)
> > +#endif
> > +
> > +
> > +#if !GLIB_CHECK_VERSION(2, 34, 0)
> > +static inline void
> > +g_test_add_data_func_full(const char *path,
> > +                          gpointer data,
> > +                          gpointer fn,
> > +                          gpointer data_free_func)
> > +{
> > +#if GLIB_CHECK_VERSION(2, 26, 0)
> > +    /* back-compat casts, remove this once we can require new-enough glib */
> > +    g_test_add_vtable(path, 0, data, NULL,
> > +                      (GTestFixtureFunc)fn, (GTestFixtureFunc) data_free_func);
> > +#else
> > +    /* back-compat casts, remove this once we can require new-enough glib */
> > +    g_test_add_vtable(path, 0, data, NULL,
> > +                      (void (*)(void)) fn, (void (*)(void)) data_free_func);
> > +#endif
> > +}
> > +#endif
> > +
> > +
> > +#endif
> > diff --git a/include/glib/glib-helper.h b/include/glib/glib-helper.h
> > new file mode 100644
> > index 0000000..db740fb
> > --- /dev/null
> > +++ b/include/glib/glib-helper.h
> > @@ -0,0 +1,30 @@
> > +/*
> > + * Helpers for GLIB
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#ifndef QEMU_GLIB_HELPER_H
> > +#define QEMU_GLIB_HELPER_H
> > +
> > +
> > +#include "glib/glib-compat.h"
> > +
> > +#define GPOINTER_TO_UINT64(a) ((guint64) (a))
> > +
> > +/*
> > + * return 1 in case of a > b, -1 otherwise and 0 if equeal
> > + */
> > +gint g_int_cmp64(gconstpointer a, gconstpointer b,
> > +        gpointer __attribute__((unused)) user_data);
> > +
> > +/*
> > + * return 1 in case of a > b, -1 otherwise and 0 if equeal
> > + */
> > +int g_int_cmp(gconstpointer a, gconstpointer b,
> > +        gpointer __attribute__((unused)) user_data);
> > +
> > +#endif /* QEMU_GLIB_HELPER_H */
> > +
> > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> > index 122ff06..36f8a89 100644
> > --- a/include/qemu/osdep.h
> > +++ b/include/qemu/osdep.h
> > @@ -104,7 +104,7 @@ extern int daemon(int, int);
> >  #include "sysemu/os-posix.h"
> >  #endif
> > 
> > -#include "glib-compat.h"
> > +#include "glib/glib-compat.h"
> >  #include "qemu/typedefs.h"
> > 
> >  #ifndef O_LARGEFILE
> > diff --git a/linux-user/main.c b/linux-user/main.c
> > index 10a3bb3..7cea6bc 100644
> > --- a/linux-user/main.c
> > +++ b/linux-user/main.c
> > @@ -35,7 +35,7 @@
> >  #include "elf.h"
> >  #include "exec/log.h"
> >  #include "trace/control.h"
> > -#include "glib-compat.h"
> > +#include "glib/glib-compat.h"
> > 
> >  char *exec_path;
> > 
> > diff --git a/scripts/clean-includes b/scripts/clean-includes
> > index dd938da..b32b928 100755
> > --- a/scripts/clean-includes
> > +++ b/scripts/clean-includes
> > @@ -123,7 +123,7 @@ for f in "$@"; do
> >        ;;
> >      *include/qemu/osdep.h | \
> >      *include/qemu/compiler.h | \
> > -    *include/glib-compat.h | \
> > +    *include/glib/glib-compat.h | \
> >      *include/sysemu/os-posix.h | \
> >      *include/sysemu/os-win32.h | \
> >      *include/standard-headers/ )
> > diff --git a/util/Makefile.objs b/util/Makefile.objs
> > index c6205eb..0080712 100644
> > --- a/util/Makefile.objs
> > +++ b/util/Makefile.objs
> > @@ -43,3 +43,4 @@ util-obj-y += qdist.o
> >  util-obj-y += qht.o
> >  util-obj-y += range.o
> >  util-obj-y += systemd.o
> > +util-obj-y += glib-helper.o
> > diff --git a/util/glib-helper.c b/util/glib-helper.c
> > new file mode 100644
> > index 0000000..2557009
> > --- /dev/null
> > +++ b/util/glib-helper.c
> > @@ -0,0 +1,29 @@
> > +/*
> > + * Implementation for GLIB helpers
> > + * this file is intented to commulate and later reuse
> > + * additional glib functions
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > +
> > + */
> > +
> > +#include "glib/glib-helper.h"
> > +
> > +gint g_int_cmp64(gconstpointer a, gconstpointer b,
> > +        gpointer __attribute__((unused)) user_data)
> > +{
> > +    guint64 ua = GPOINTER_TO_UINT64(a);
> > +    guint64 ub = GPOINTER_TO_UINT64(b);
> > +    return (ua > ub) - (ua < ub);
> > +}
> > +
> > +/*
> > + * return 1 in case of a > b, -1 otherwise and 0 if equeal
> > + */
> > +gint g_int_cmp(gconstpointer a, gconstpointer b,
> > +        gpointer __attribute__((unused)) user_data)
> > +{
> > +    return g_int_cmp64(a, b, user_data);
> > +}
> > +
> > 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 2/6] util: introduce glib-helper.c
Posted by Peter Maydell 8 years, 9 months ago
On 14 April 2017 at 14:17, Alexey Perevalov <a.perevalov@samsung.com> wrote:
> There is a lack of g_int_cmp which compares pointers value in glib,
> xen_disk.c introduced its own, so the same function now requires
> in migration.c. So logically to move it into common place.
> Futher: maybe extend glib.
>
> Also this commit moves existing glib-compat.h into util/glib
> folder for consolidation purpose.
>
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>

Hi; thanks for this patch. I have some comments below, mostly
aimed at improving the documentation in comments of what these
new header files and functions are for -- the bar for "how
much explanation do we need" moves up when a function is
moved from being local to a single file to being available
to all of QEMU.

> diff --git a/include/glib/glib-helper.h b/include/glib/glib-helper.h
> new file mode 100644
> index 0000000..db740fb
> --- /dev/null
> +++ b/include/glib/glib-helper.h
> @@ -0,0 +1,30 @@
> +/*
> + * Helpers for GLIB
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */

So glib-compat.h is for functions which exist in newer versions
of glib but not older ones. What's this header for? Ideally the
comment at the top of the file should make it clear what kinds
of functions go here rather than elsewhere.

Also, GLib is capitalized like that, and you should have a
Copyright line here.

> +
> +#ifndef QEMU_GLIB_HELPER_H
> +#define QEMU_GLIB_HELPER_H
> +
> +
> +#include "glib/glib-compat.h"

Nothing needs to include glib-compat.h directly, because osdep.h does.

> +
> +#define GPOINTER_TO_UINT64(a) ((guint64) (a))
> +
> +/*
> + * return 1 in case of a > b, -1 otherwise and 0 if equeal
> + */

Can we have a proper doc comment format comment, please,
since this is now a function available to all of QEMU?

> +gint g_int_cmp64(gconstpointer a, gconstpointer b,
> +        gpointer __attribute__((unused)) user_data);

What is this actually for? Looking at the original uses
I can tell that this is a GCompareDataFunc function, but
the comment should tell me that.

It also looks very fishy because the function name suggests
a 64 bit compare but gconstpointer may only be 32 bits.

I'm not sure it makes sense to specify the unused attribute
on the function prototype -- that is a property of the
implementation, not of the API exposed to callers, so it
should go on the function definition IMHO.

> +
> +/*
> + * return 1 in case of a > b, -1 otherwise and 0 if equeal
> + */

This is the same comment as above, so it doesn't explain
what the difference between the two functions is.

> +int g_int_cmp(gconstpointer a, gconstpointer b,
> +        gpointer __attribute__((unused)) user_data);
> +
> +#endif /* QEMU_GLIB_HELPER_H */
> +
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 122ff06..36f8a89 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -104,7 +104,7 @@ extern int daemon(int, int);
>  #include "sysemu/os-posix.h"
>  #endif
>
> -#include "glib-compat.h"
> +#include "glib/glib-compat.h"
>  #include "qemu/typedefs.h"
>
>  #ifndef O_LARGEFILE
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 10a3bb3..7cea6bc 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -35,7 +35,7 @@
>  #include "elf.h"
>  #include "exec/log.h"
>  #include "trace/control.h"
> -#include "glib-compat.h"
> +#include "glib/glib-compat.h"

osdep.h includes glib-compat.h so we should just delete the #include,
not change it.

This patch looks like it will break bsd-user compiles, because
bsd-user/main.c has the same unnecessary glib-compat.h include
and the patch doesn't change or delete it.

>
>  char *exec_path;
>
> diff --git a/scripts/clean-includes b/scripts/clean-includes
> index dd938da..b32b928 100755
> --- a/scripts/clean-includes
> +++ b/scripts/clean-includes
> @@ -123,7 +123,7 @@ for f in "$@"; do
>        ;;
>      *include/qemu/osdep.h | \
>      *include/qemu/compiler.h | \
> -    *include/glib-compat.h | \
> +    *include/glib/glib-compat.h | \
>      *include/sysemu/os-posix.h | \
>      *include/sysemu/os-win32.h | \
>      *include/standard-headers/ )
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index c6205eb..0080712 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -43,3 +43,4 @@ util-obj-y += qdist.o
>  util-obj-y += qht.o
>  util-obj-y += range.o
>  util-obj-y += systemd.o
> +util-obj-y += glib-helper.o
> diff --git a/util/glib-helper.c b/util/glib-helper.c
> new file mode 100644
> index 0000000..2557009
> --- /dev/null
> +++ b/util/glib-helper.c
> @@ -0,0 +1,29 @@
> +/*
> + * Implementation for GLIB helpers
> + * this file is intented to commulate and later reuse
> + * additional glib functions

Did you mean "accumulate" ?

More detailed description of what functions live in this
file would be useful -- these aren't actually GLib
functions, just utility routines that are useful to
code which uses GLib, as far as I can tell.

> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> +

Stray blank line.

> + */

This is also missing the copyright line.

> +
> +#include "glib/glib-helper.h"

Every C file should start by including "qemu/osdep.h" as the
first thing it does.

> +
> +gint g_int_cmp64(gconstpointer a, gconstpointer b,
> +        gpointer __attribute__((unused)) user_data)
> +{
> +    guint64 ua = GPOINTER_TO_UINT64(a);
> +    guint64 ub = GPOINTER_TO_UINT64(b);
> +    return (ua > ub) - (ua < ub);
> +}
> +
> +/*
> + * return 1 in case of a > b, -1 otherwise and 0 if equeal
> + */
> +gint g_int_cmp(gconstpointer a, gconstpointer b,
> +        gpointer __attribute__((unused)) user_data)
> +{
> +    return g_int_cmp64(a, b, user_data);
> +}
> +
> --
> 1.8.3.1
>
>

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/6] util: introduce glib-helper.c
Posted by Alexey 8 years, 9 months ago
Hello, thank you for so  detailed comment,

On Fri, Apr 21, 2017 at 11:27:55AM +0100, Peter Maydell wrote:
> On 14 April 2017 at 14:17, Alexey Perevalov <a.perevalov@samsung.com> wrote:
> > There is a lack of g_int_cmp which compares pointers value in glib,
> > xen_disk.c introduced its own, so the same function now requires
> > in migration.c. So logically to move it into common place.
> > Futher: maybe extend glib.
> >
> > Also this commit moves existing glib-compat.h into util/glib
> > folder for consolidation purpose.
> >
> > Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> 
> Hi; thanks for this patch. I have some comments below, mostly
> aimed at improving the documentation in comments of what these
> new header files and functions are for -- the bar for "how
> much explanation do we need" moves up when a function is
> moved from being local to a single file to being available
> to all of QEMU.
> 
> > diff --git a/include/glib/glib-helper.h b/include/glib/glib-helper.h
> > new file mode 100644
> > index 0000000..db740fb
> > --- /dev/null
> > +++ b/include/glib/glib-helper.h
> > @@ -0,0 +1,30 @@
> > +/*
> > + * Helpers for GLIB
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> 
> So glib-compat.h is for functions which exist in newer versions
> of glib but not older ones. What's this header for? Ideally the
> comment at the top of the file should make it clear what kinds
> of functions go here rather than elsewhere.
> 
> Also, GLib is capitalized like that, and you should have a
> Copyright line here.
> 
> > +
> > +#ifndef QEMU_GLIB_HELPER_H
> > +#define QEMU_GLIB_HELPER_H
> > +
> > +
> > +#include "glib/glib-compat.h"
> 
> Nothing needs to include glib-compat.h directly, because osdep.h does.
> 
> > +
> > +#define GPOINTER_TO_UINT64(a) ((guint64) (a))
> > +
> > +/*
> > + * return 1 in case of a > b, -1 otherwise and 0 if equeal
> > + */
> 
> Can we have a proper doc comment format comment, please,
> since this is now a function available to all of QEMU?
> 
> > +gint g_int_cmp64(gconstpointer a, gconstpointer b,
> > +        gpointer __attribute__((unused)) user_data);
> 
> What is this actually for? Looking at the original uses
> I can tell that this is a GCompareDataFunc function, but
> the comment should tell me that.
I looked at another functions comments in QEMU, I didn't find
some common style, and decided keep it as is. Maybe I omitted some
best practice here.


> 
> It also looks very fishy because the function name suggests
> a 64 bit compare but gconstpointer may only be 32 bits.
> 
> I'm not sure it makes sense to specify the unused attribute
> on the function prototype -- that is a property of the
> implementation, not of the API exposed to callers, so it
> should go on the function definition IMHO.
> 
> > +
> > +/*
> > + * return 1 in case of a > b, -1 otherwise and 0 if equeal
> > + */
> 
> This is the same comment as above, so it doesn't explain
> what the difference between the two functions is.
> 

yes, it was copy pasted,
right now, after mingw build check I think to use intptr_t as a type
for comparision in this function or even keep gpointer and merge these two
functions into _direct_.
I saw intptr_t is widely used in QEMU.

The intent of this function was a comparator for case when client code
want to keep integers in pointer field. xen_disk.c uses UINT32 so it
wasn't a problem, but migration uses 64 address (kernel provides it in
__u64, long long), so on 32 platform it's a problem.
Fortunately userfaultfd handler is linux specific code, 
and I'm going to keep there just cast, like that GUINT_TO_POINTER

#define GUINT_TO_POINTER(u)     ((gpointer) ${glib_gpui_cast} (u))

on 64 architecture glib_gpui_cast is guint64.

> > +int g_int_cmp(gconstpointer a, gconstpointer b,
> > +        gpointer __attribute__((unused)) user_data);
> > +
> > +#endif /* QEMU_GLIB_HELPER_H */
> > +
> > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> > index 122ff06..36f8a89 100644
> > --- a/include/qemu/osdep.h
> > +++ b/include/qemu/osdep.h
> > @@ -104,7 +104,7 @@ extern int daemon(int, int);
> >  #include "sysemu/os-posix.h"
> >  #endif
> >
> > -#include "glib-compat.h"
> > +#include "glib/glib-compat.h"
> >  #include "qemu/typedefs.h"
> >
> >  #ifndef O_LARGEFILE
> > diff --git a/linux-user/main.c b/linux-user/main.c
> > index 10a3bb3..7cea6bc 100644
> > --- a/linux-user/main.c
> > +++ b/linux-user/main.c
> > @@ -35,7 +35,7 @@
> >  #include "elf.h"
> >  #include "exec/log.h"
> >  #include "trace/control.h"
> > -#include "glib-compat.h"
> > +#include "glib/glib-compat.h"
> 
> osdep.h includes glib-compat.h so we should just delete the #include,
> not change it.
> 
> This patch looks like it will break bsd-user compiles, because
> bsd-user/main.c has the same unnecessary glib-compat.h include
> and the patch doesn't change or delete it.
> 
> >
> >  char *exec_path;
> >
> > diff --git a/scripts/clean-includes b/scripts/clean-includes
> > index dd938da..b32b928 100755
> > --- a/scripts/clean-includes
> > +++ b/scripts/clean-includes
> > @@ -123,7 +123,7 @@ for f in "$@"; do
> >        ;;
> >      *include/qemu/osdep.h | \
> >      *include/qemu/compiler.h | \
> > -    *include/glib-compat.h | \
> > +    *include/glib/glib-compat.h | \
> >      *include/sysemu/os-posix.h | \
> >      *include/sysemu/os-win32.h | \
> >      *include/standard-headers/ )
> > diff --git a/util/Makefile.objs b/util/Makefile.objs
> > index c6205eb..0080712 100644
> > --- a/util/Makefile.objs
> > +++ b/util/Makefile.objs
> > @@ -43,3 +43,4 @@ util-obj-y += qdist.o
> >  util-obj-y += qht.o
> >  util-obj-y += range.o
> >  util-obj-y += systemd.o
> > +util-obj-y += glib-helper.o
> > diff --git a/util/glib-helper.c b/util/glib-helper.c
> > new file mode 100644
> > index 0000000..2557009
> > --- /dev/null
> > +++ b/util/glib-helper.c
> > @@ -0,0 +1,29 @@
> > +/*
> > + * Implementation for GLIB helpers
> > + * this file is intented to commulate and later reuse
> > + * additional glib functions
> 
> Did you mean "accumulate" ?
> 
> More detailed description of what functions live in this
> file would be useful -- these aren't actually GLib
> functions, just utility routines that are useful to
> code which uses GLib, as far as I can tell.
> 
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > +
> 
> Stray blank line.
> 
> > + */
> 
> This is also missing the copyright line.
Yes, maybe it was better for me to ask before send.
I found in util files with reference to GNU GPL, version 2, like
in this file, also I found that

 * This library is free software; you can redistribute it and/or
 * modify it under the terms of the GNU Lesser General Public
 * License as published by the Free Software Foundation; either
 * version 2 of the License, or (at your option) any later version.

So I just copied copyright reference from glib-compat.h.

> 
> > +
> > +#include "glib/glib-helper.h"
> 
> Every C file should start by including "qemu/osdep.h" as the
> first thing it does.
> 
> > +
> > +gint g_int_cmp64(gconstpointer a, gconstpointer b,
> > +        gpointer __attribute__((unused)) user_data)
> > +{
> > +    guint64 ua = GPOINTER_TO_UINT64(a);
> > +    guint64 ub = GPOINTER_TO_UINT64(b);
> > +    return (ua > ub) - (ua < ub);
> > +}
> > +
> > +/*
> > + * return 1 in case of a > b, -1 otherwise and 0 if equeal
> > + */
> > +gint g_int_cmp(gconstpointer a, gconstpointer b,
> > +        gpointer __attribute__((unused)) user_data)
> > +{
> > +    return g_int_cmp64(a, b, user_data);
> > +}
> > +
> > --
> > 1.8.3.1
> >
> >
> 
> thanks
> -- PMM
> 

-- 

BR
Alexey

Re: [Qemu-devel] [PATCH 2/6] util: introduce glib-helper.c
Posted by Peter Maydell 8 years, 9 months ago
On 21 April 2017 at 16:10, Alexey <a.perevalov@samsung.com> wrote:
> Hello, thank you for so  detailed comment,
>
> On Fri, Apr 21, 2017 at 11:27:55AM +0100, Peter Maydell wrote:

>> Can we have a proper doc comment format comment, please,
>> since this is now a function available to all of QEMU?
>>
>> > +gint g_int_cmp64(gconstpointer a, gconstpointer b,
>> > +        gpointer __attribute__((unused)) user_data);
>>
>> What is this actually for? Looking at the original uses
>> I can tell that this is a GCompareDataFunc function, but
>> the comment should tell me that.
> I looked at another functions comments in QEMU, I didn't find
> some common style, and decided keep it as is. Maybe I omitted some
> best practice here.

See include/qemu/bitops.h for an example of the comment style.
More important than just the style is that the comment
should clearly explain the purpose of the function in detail.

Certainly many of our existing functions are poorly documented,
but we're trying to raise the bar gradually here.

> yes, it was copy pasted,
> right now, after mingw build check I think to use intptr_t as a type
> for comparision in this function or even keep gpointer and merge these two
> functions into _direct_.
> I saw intptr_t is widely used in QEMU.
>
> The intent of this function was a comparator for case when client code
> want to keep integers in pointer field. xen_disk.c uses UINT32 so it
> wasn't a problem, but migration uses 64 address (kernel provides it in
> __u64, long long), so on 32 platform it's a problem.

Code which tries to put a genuinely 64 bit value into a pointer
is buggy and needs to be fixed. I'm not clear if that is the
case here, or if the ABI from the kernel guarantees that the
value is really a pointer type and fits in uintptr_t / gpointer.

I don't think we need more than one of these functions.

>> This is also missing the copyright line.
> Yes, maybe it was better for me to ask before send.
> I found in util files with reference to GNU GPL, version 2, like
> in this file, also I found that
>
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Lesser General Public
>  * License as published by the Free Software Foundation; either
>  * version 2 of the License, or (at your option) any later version.
>
> So I just copied copyright reference from glib-compat.h.

Yes, that's the license statement, which is fine. What is
missing is the copyright line, which in glib-compat.h looks
like:
 Copyright IBM, Corp. 2013

For code you write, you want either your personal or (more likely)
a Samsung copyright line -- check with your company about what
their preferred form is.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/6] util: introduce glib-helper.c
Posted by Dr. David Alan Gilbert 8 years, 9 months ago
* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 21 April 2017 at 16:10, Alexey <a.perevalov@samsung.com> wrote:
> > Hello, thank you for so  detailed comment,
> >
> > On Fri, Apr 21, 2017 at 11:27:55AM +0100, Peter Maydell wrote:
> 
> >> Can we have a proper doc comment format comment, please,
> >> since this is now a function available to all of QEMU?
> >>
> >> > +gint g_int_cmp64(gconstpointer a, gconstpointer b,
> >> > +        gpointer __attribute__((unused)) user_data);
> >>
> >> What is this actually for? Looking at the original uses
> >> I can tell that this is a GCompareDataFunc function, but
> >> the comment should tell me that.
> > I looked at another functions comments in QEMU, I didn't find
> > some common style, and decided keep it as is. Maybe I omitted some
> > best practice here.
> 
> See include/qemu/bitops.h for an example of the comment style.
> More important than just the style is that the comment
> should clearly explain the purpose of the function in detail.
> 
> Certainly many of our existing functions are poorly documented,
> but we're trying to raise the bar gradually here.
> 
> > yes, it was copy pasted,
> > right now, after mingw build check I think to use intptr_t as a type
> > for comparision in this function or even keep gpointer and merge these two
> > functions into _direct_.
> > I saw intptr_t is widely used in QEMU.
> >
> > The intent of this function was a comparator for case when client code
> > want to keep integers in pointer field. xen_disk.c uses UINT32 so it
> > wasn't a problem, but migration uses 64 address (kernel provides it in
> > __u64, long long), so on 32 platform it's a problem.
> 
> Code which tries to put a genuinely 64 bit value into a pointer
> is buggy and needs to be fixed. I'm not clear if that is the
> case here, or if the ABI from the kernel guarantees that the
> value is really a pointer type and fits in uintptr_t / gpointer.

It's a (probably masked) HVA, so always a valid pointer.

Dave

> I don't think we need more than one of these functions.
> 
> >> This is also missing the copyright line.
> > Yes, maybe it was better for me to ask before send.
> > I found in util files with reference to GNU GPL, version 2, like
> > in this file, also I found that
> >
> >  * This library is free software; you can redistribute it and/or
> >  * modify it under the terms of the GNU Lesser General Public
> >  * License as published by the Free Software Foundation; either
> >  * version 2 of the License, or (at your option) any later version.
> >
> > So I just copied copyright reference from glib-compat.h.
> 
> Yes, that's the license statement, which is fine. What is
> missing is the copyright line, which in glib-compat.h looks
> like:
>  Copyright IBM, Corp. 2013
> 
> For code you write, you want either your personal or (more likely)
> a Samsung copyright line -- check with your company about what
> their preferred form is.
> 
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK