[virt-tools-list] [virt-viewer 4/4] ovirt: Improve handling of g_strv_contains()

[Eduardo Lima (Etrunko)]

On 14/06/18 13:01, Christophe Fergeau wrote:
> The ovirt code uses g_strv_contains() with fallback code in
> glib-compat.h when we are using a glib version where it's not available.
> However, when we use a glib version where g_strv_contains is available,
> we get a compilation warning since we are compiling GLIB_VERSION_MAX_ALLOWED
> set to 2.38.
> 
> This commit wraps both the compat code and the g_strv_contains() call in
> a strv_contains() helper where we can hide the magic needed in the 2
> cases to avoid compilation issues.
> 
> Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> ---
>  src/Makefile.am          |  2 --
>  src/glib-compat.c        | 35 -----------------------------------
>  src/glib-compat.h        | 39 ---------------------------------------
>  src/ovirt-foreign-menu.c | 22 ++++++++++++++++++++--
>  4 files changed, 20 insertions(+), 78 deletions(-)
>  delete mode 100644 src/glib-compat.c
>  delete mode 100644 src/glib-compat.h
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 0a3cbbf..f02fdb4 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -54,8 +54,6 @@ libvirt_viewer_util_la_SOURCES = \
>  
>  libvirt_viewer_la_SOURCES =					\
>  	$(BUILT_SOURCES)				\
> -	glib-compat.h					\
> -	glib-compat.c					\
>  	virt-viewer-auth.h				\
>  	virt-viewer-auth.c				\
>  	virt-viewer-app.h				\
> diff --git a/src/glib-compat.c b/src/glib-compat.c
> deleted file mode 100644
> index 62ac87e..0000000
> --- a/src/glib-compat.c
> +++ /dev/null
> @@ -1,35 +0,0 @@
> -/*
> - * 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.
> - *
> - * This library is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - * Lesser General Public License for more details.
> - *
> - * You should have received a copy of the GNU Lesser General Public
> - * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> - */
> -#include <config.h>
> -
> -#include "glib-compat.h"
> -
> -#if !GLIB_CHECK_VERSION(2,44,0)
> -gboolean
> -g_strv_contains (const gchar * const *strv,
> -                 const gchar         *str)
> -{
> -  g_return_val_if_fail (strv != NULL, FALSE);
> -  g_return_val_if_fail (str != NULL, FALSE);
> -
> -  for (; *strv != NULL; strv++)
> -    {
> -      if (g_str_equal (str, *strv))
> -        return TRUE;
> -    }
> -
> -  return FALSE;
> -}
> -#endif
> diff --git a/src/glib-compat.h b/src/glib-compat.h
> deleted file mode 100644
> index f1b43ae..0000000
> --- a/src/glib-compat.h
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -/*
> - * Virt Viewer: A virtual machine console viewer
> - *
> - * Copyright (C) 2017 Red Hat, Inc.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> - *
> - * Author: Eduardo Lima (Etrunko) <etrunko at redhat.com>
> - */
> -
> -#include <config.h>
> -
> -#ifndef GLIB_COMPAT_H
> -#define GLIB_COMPAT_H 1
> -
> -#include <glib.h>
> -
> -G_BEGIN_DECLS
> -
> -#if !GLIB_CHECK_VERSION(2,44,0)
> -gboolean              g_strv_contains  (const gchar * const *strv,
> -                                        const gchar         *str);
> -#endif
> -
> -G_END_DECLS
> -
> -#endif // GLIB_COMPAT_H
> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
> index cd1b8bd..8650d0c 100644
> --- a/src/ovirt-foreign-menu.c
> +++ b/src/ovirt-foreign-menu.c
> @@ -29,7 +29,6 @@
>  
>  #include "ovirt-foreign-menu.h"
>  #include "virt-viewer-util.h"
> -#include "glib-compat.h"
>  
>  typedef enum {
>      STATE_0,
> @@ -100,6 +99,25 @@ enum {
>      PROP_VM_GUID,
>  };
>  
> +static gboolean strv_contains(const gchar * const *strv, const gchar *str)
> +{
> +#if GLIB_CHECK_VERSION(2,44,0)
> +G_GNUC_BEGIN_IGNORE_DEPRECATIONS
> +  return g_strv_contains (strv, str);
> +G_GNUC_END_IGNORE_DEPRECATIONS
> +#else
> +  g_return_val_if_fail (strv != NULL, FALSE);
> +  g_return_val_if_fail (str != NULL, FALSE);
> +
> +  for (; *strv != NULL; strv++)
> +    {
> +      if (g_str_equal (str, *strv))
> +        return TRUE;
> +    }
> +
> +  return FALSE;
> +#endif
> +}
>  

Hmmm, Not sure about moving the function to this file, while you could
keep things as they currently are and simply surround the
g_strv_contains() call like the following:


G_GNUC_BEGIN_IGNORE_DEPRECATIONS
  ... g_strv_contains();
G_GNUC_END_IGNORE_DEPRECATIONS


What do you think?

>  gchar *
>  ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu)
> @@ -632,7 +650,7 @@ static gboolean storage_domain_attached_to_data_center(OvirtStorageDomain *domai
>  
>      g_object_get(domain, "data-center-ids", &data_center_ids, NULL);
>      g_object_get(data_center, "guid", &data_center_guid, NULL);
> -    match = g_strv_contains((const gchar * const *) data_center_ids, data_center_guid);
> +    match = strv_contains((const gchar * const *) data_center_ids, data_center_guid);
>      g_strfreev(data_center_ids);
>      g_free(data_center_guid);
>  
> 


-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko at redhat.com