[libvirt] [PATCH 1/7] util: always replace g_fsync usage with our wrapper

Daniel P. Berrangé posted 7 patches 5 years, 11 months ago
[libvirt] [PATCH 1/7] util: always replace g_fsync usage with our wrapper
Posted by Daniel P. Berrangé 5 years, 11 months ago
g_fsync was introduced in 2.63 which is newer than our minimum
glib version. A future commit will introduce compile time
checking of API versions to prevent accidental usage of APIs
from glib newer than our min declared.

To avoid triggering this warning, however, we need to ensure
that we always use our wrapper function via glibcompat.c,
which will disable the API version warnings.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/util/glibcompat.c | 1 +
 src/util/glibcompat.h | 5 ++---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c
index 4ebefb4478..9fba54cb79 100644
--- a/src/util/glibcompat.c
+++ b/src/util/glibcompat.c
@@ -55,6 +55,7 @@ vir_g_strdup_vprintf(const char *msg, va_list args)
 }
 
 
+/* Drop when min glib >= 2.63.0 */
 gint
 vir_g_fsync(gint fd)
 {
diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h
index d6b83f4b93..7878ad24ed 100644
--- a/src/util/glibcompat.h
+++ b/src/util/glibcompat.h
@@ -32,6 +32,5 @@ gint vir_g_fsync(gint fd);
 # define g_strdup_vprintf vir_g_strdup_vprintf
 #endif
 
-#if !GLIB_CHECK_VERSION(2, 63, 0)
-# define g_fsync vir_g_fsync
-#endif
+#undef g_fsync
+#define g_fsync vir_g_fsync
-- 
2.24.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/7] util: always replace g_fsync usage with our wrapper
Posted by Pavel Hrdina 5 years, 11 months ago
On Mon, Jan 06, 2020 at 05:26:49PM +0000, Daniel P. Berrangé wrote:
> g_fsync was introduced in 2.63 which is newer than our minimum
> glib version. A future commit will introduce compile time
> checking of API versions to prevent accidental usage of APIs
> from glib newer than our min declared.
> 
> To avoid triggering this warning, however, we need to ensure
> that we always use our wrapper function via glibcompat.c,
> which will disable the API version warnings.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/util/glibcompat.c | 1 +
>  src/util/glibcompat.h | 5 ++---
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c
> index 4ebefb4478..9fba54cb79 100644
> --- a/src/util/glibcompat.c
> +++ b/src/util/glibcompat.c
> @@ -55,6 +55,7 @@ vir_g_strdup_vprintf(const char *msg, va_list args)
>  }
>  
>  
> +/* Drop when min glib >= 2.63.0 */

These "drop when something >= version" are usually overlooked and stick
around longer then necessary, should we add a check that would produce
compile time warning if we bump the minimal version of glib?

Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/7] util: always replace g_fsync usage with our wrapper
Posted by Daniel P. Berrangé 5 years, 11 months ago
On Tue, Jan 07, 2020 at 09:08:03AM +0100, Pavel Hrdina wrote:
> On Mon, Jan 06, 2020 at 05:26:49PM +0000, Daniel P. Berrangé wrote:
> > g_fsync was introduced in 2.63 which is newer than our minimum
> > glib version. A future commit will introduce compile time
> > checking of API versions to prevent accidental usage of APIs
> > from glib newer than our min declared.
> > 
> > To avoid triggering this warning, however, we need to ensure
> > that we always use our wrapper function via glibcompat.c,
> > which will disable the API version warnings.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  src/util/glibcompat.c | 1 +
> >  src/util/glibcompat.h | 5 ++---
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c
> > index 4ebefb4478..9fba54cb79 100644
> > --- a/src/util/glibcompat.c
> > +++ b/src/util/glibcompat.c
> > @@ -55,6 +55,7 @@ vir_g_strdup_vprintf(const char *msg, va_list args)
> >  }
> >  
> >  
> > +/* Drop when min glib >= 2.63.0 */
> 
> These "drop when something >= version" are usually overlooked and stick
> around longer then necessary, should we add a check that would produce
> compile time warning if we bump the minimal version of glib?

I don't think that's a real problem with the way we do things here. The
key value of having a glibcompat.{c,h} file is that it isolates the
conditional logic in one place, so when bumping min glib we only need
scan the glibcompat.{c,h} files for things to eliminate.  So chance of
missing this is minimal.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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