[v2,1/2] glib: add compatibility implementation for g_dir_make_tmp()

Commit Message

Peter Maydell Aug. 19, 2016, 11:45 a.m. UTC

On 18 August 2016 at 19:46, Sascha Silbe <[e-mail]> wrote:
> We're going to make use of g_dir_make_tmp() in test-logging. Provide a
> compatibility implementation of it for glib < 2.30.
>
> May behave differently in some edge cases (e.g. pattern only at the
> end of the template, the file name is not part of the error message),
> but good enough in practice.
>
> Signed-off-by: Sascha Silbe <[e-mail]>
> ---
> v1→v2:
>   - new patch
>
>  include/glib-compat.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/include/glib-compat.h b/include/glib-compat.h
> index 01aa7b3..de64ac2 100644
> --- a/include/glib-compat.h
> +++ b/include/glib-compat.h
> @@ -48,6 +48,27 @@ static inline gint64 qemu_g_get_monotonic_time(void)
>  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 const *template = tmpl ? tmpl : ".XXXXXX";

This doesn't build:
In file included from /Users/pm215/src/qemu-for-merges/disas/arm-a64.cc:21:
In file included from /Users/pm215/src/qemu-for-merges/include/qemu/osdep.h:107:
/Users/pm215/src/qemu-for-merges/include/glib-compat.h:57:18: error:
expected unqualified-id
    gchar const *template = tmpl ? tmpl : ".XXXXXX";
                 ^
/Users/pm215/src/qemu-for-merges/include/glib-compat.h:58:53: error:
expected expression
    gchar *path = g_build_filename(g_get_tmp_dir(), template, NULL);
                                                    ^

because this header is included from some C++ files, where
"template" is a reserved word. I've fixed this by squashing
in the following change:


thanks
-- PMM

Comments

Sascha Silbe Aug. 23, 2016, 1:21 p.m. UTC | #1

Dear Peter,

Peter Maydell <[e-mail]> writes:

[...]
>> +#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 const *template = tmpl ? tmpl : ".XXXXXX";
>
> This doesn't build:
> In file included from /Users/pm215/src/qemu-for-merges/disas/arm-a64.cc:21:
> In file included from /Users/pm215/src/qemu-for-merges/include/qemu/osdep.h:107:
> /Users/pm215/src/qemu-for-merges/include/glib-compat.h:57:18: error:
> expected unqualified-id
>     gchar const *template = tmpl ? tmpl : ".XXXXXX";
>                  ^
> /Users/pm215/src/qemu-for-merges/include/glib-compat.h:58:53: error:
> expected expression
>     gchar *path = g_build_filename(g_get_tmp_dir(), template, NULL);
>                                                     ^
>
> because this header is included from some C++ files, where
> "template" is a reserved word. I've fixed this by squashing
> in the following change:

Curious, didn't hit this on my laptop (Ubuntu 14.04, x86_64) and neither
in the centos6 docker image. At least on my laptop arm-a64.cc gets
compiled and -Werror is enabled, too. Does this happen with a particular
tool chain only? clang perchance?

FWIW, I would have expected the C language linkage specification
(i.e. extern "C" {}) in disas/arm-a64.cc to take care of this kind of
issue. However after scanning through C++11 it seems the linkage
specification only affects external linkage names. Plus is doesn't seem
keywords are valid identifiers even for external C language linkage
names; there's only an exception for attributes, not for non-C++
language linkage.


[From second mail]
> Applied to master (with minor fixup to patch 1); thanks.

Thanks!

Sascha

Patch

--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -54,8 +54,7 @@  gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
  * template. */
 static inline gchar *qemu_g_dir_make_tmp(gchar const *tmpl, GError **error)
 {
-    gchar const *template = tmpl ? tmpl : ".XXXXXX";
-    gchar *path = g_build_filename(g_get_tmp_dir(), template, NULL);
+    gchar *path = g_build_filename(g_get_tmp_dir(), tmpl ?: ".XXXXXX", NULL);

     if (mkdtemp(path) != NULL) {
         return path;