[PATCH 0/2] meson: Declare GLIB_VERSION_* macros at configure

I was playing around with bumped glib version and found couple of issues. Michal Prívozník (2): qemu_domainjob: Drop 'const' from strings in _qemuDomainJobObj meson: Declare GLIB_VERSION_* macros at configure config.h | 10 ---------- meson.build | 7 +++++++ src/qemu/qemu_domainjob.h | 6 +++--- 3 files changed, 10 insertions(+), 13 deletions(-) -- 2.26.3

These strings are not constant really. They are allocated in qemuDomainObjBeginJobInternal() and freed in qemuDomainReset*Job(). Freeing a pointer to const looks weird. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domainjob.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domainjob.h b/src/qemu/qemu_domainjob.h index 75cf1deb6a..46cbb8a067 100644 --- a/src/qemu/qemu_domainjob.h +++ b/src/qemu/qemu_domainjob.h @@ -179,20 +179,20 @@ struct _qemuDomainJobObj { /* The following members are for QEMU_JOB_* */ qemuDomainJob active; /* Currently running job */ unsigned long long owner; /* Thread id which set current job */ - const char *ownerAPI; /* The API which owns the job */ + char *ownerAPI; /* The API which owns the job */ unsigned long long started; /* When the current job started */ /* The following members are for QEMU_AGENT_JOB_* */ qemuDomainAgentJob agentActive; /* Currently running agent job */ unsigned long long agentOwner; /* Thread id which set current agent job */ - const char *agentOwnerAPI; /* The API which owns the agent job */ + char *agentOwnerAPI; /* The API which owns the agent job */ unsigned long long agentStarted; /* When the current agent job started */ /* The following members are for QEMU_ASYNC_JOB_* */ virCond asyncCond; /* Use to coordinate with async jobs */ qemuDomainAsyncJob asyncJob; /* Currently active async job */ unsigned long long asyncOwner; /* Thread which set current async job */ - const char *asyncOwnerAPI; /* The API which owns the async job */ + char *asyncOwnerAPI; /* The API which owns the async job */ unsigned long long asyncStarted; /* When the current async job started */ int phase; /* Job phase (mainly for migrations) */ unsigned long long mask; /* Jobs allowed during async job */ -- 2.26.3

On Fri, Apr 30, 2021 at 07:18:48AM +0200, Michal Privoznik wrote:
These strings are not constant really. They are allocated in qemuDomainObjBeginJobInternal() and freed in qemuDomainReset*Job(). Freeing a pointer to const looks weird.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domainjob.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

So far we have three places where glib version is recorded: meson.build and then in config.h. The latter is so well hidden that it's easy to miss when bumping minimal glib version in the former. With a bit of python^Wmeson string magic GLIB_VERSION_MIN_REQUIRED and GLIB_VERSION_MAX_ALLOWED macros can be defined to match glib_version from meson.build. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- config.h | 10 ---------- meson.build | 7 +++++++ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/config.h b/config.h index ca6720f37d..0eacfd139d 100644 --- a/config.h +++ b/config.h @@ -51,13 +51,3 @@ #else # error You either need at least GCC 4.8 or Clang 3.4 or XCode Clang 5.1 to compile libvirt #endif - -/* Ask for warnings for anything that was marked deprecated in - * the defined version, or before. It is a candidate for rewrite. - */ -#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_48 - -/* Ask for warnings if code tries to use function that did not - * exist in the defined version. These risk breaking builds - */ -#define GLIB_VERSION_MAX_ALLOWED GLIB_VERSION_2_48 diff --git a/meson.build b/meson.build index 951da67896..597e5d8a13 100644 --- a/meson.build +++ b/meson.build @@ -949,6 +949,13 @@ endif glib_dep = declare_dependency( dependencies: [ glib_dep, gobject_dep, gio_dep ], ) +glib_version_arr=glib_version.split('.') +# Ask for warnings for anything that was marked deprecated in +# the defined version, or before. It is a candidate for rewrite. +conf.set('GLIB_VERSION_MIN_REQUIRED', 'GLIB_VERSION_' + glib_version_arr[0] + '_' + glib_version_arr[1]) +# Ask for warnings if code tries to use function that did not +# exist in the defined version. These risk breaking builds +conf.set('GLIB_VERSION_MAX_ALLOWED', 'GLIB_VERSION_' + glib_version_arr[0] + '_' + glib_version_arr[1]) glusterfs_version = '3.4.1' glusterfs_dep = dependency('glusterfs-api', version: '>=' + glusterfs_version, required: get_option('glusterfs')) -- 2.26.3

On Fri, Apr 30, 2021 at 07:18:49AM +0200, Michal Privoznik wrote:
So far we have three places where glib version is recorded: meson.build and then in config.h. The latter is so well hidden that it's easy to miss when bumping minimal glib version in the former. With a bit of python^Wmeson string magic GLIB_VERSION_MIN_REQUIRED and GLIB_VERSION_MAX_ALLOWED macros can be defined to match glib_version from meson.build.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- config.h | 10 ---------- meson.build | 7 +++++++ 2 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/config.h b/config.h index ca6720f37d..0eacfd139d 100644 --- a/config.h +++ b/config.h @@ -51,13 +51,3 @@ #else # error You either need at least GCC 4.8 or Clang 3.4 or XCode Clang 5.1 to compile libvirt #endif - -/* Ask for warnings for anything that was marked deprecated in - * the defined version, or before. It is a candidate for rewrite. - */ -#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_48 - -/* Ask for warnings if code tries to use function that did not - * exist in the defined version. These risk breaking builds - */ -#define GLIB_VERSION_MAX_ALLOWED GLIB_VERSION_2_48 diff --git a/meson.build b/meson.build index 951da67896..597e5d8a13 100644 --- a/meson.build +++ b/meson.build @@ -949,6 +949,13 @@ endif glib_dep = declare_dependency( dependencies: [ glib_dep, gobject_dep, gio_dep ], ) +glib_version_arr=glib_version.split('.')
s/=/ = / In the spirit of removing duplicated code how about having another variable: glib_version_str = 'GLIB_VERSION_@0@_@1@'.format(glib_version_arr[0], glib_version_arr[1]) and using it in the following code?
+# Ask for warnings for anything that was marked deprecated in +# the defined version, or before. It is a candidate for rewrite. +conf.set('GLIB_VERSION_MIN_REQUIRED', 'GLIB_VERSION_' + glib_version_arr[0] + '_' + glib_version_arr[1]) +# Ask for warnings if code tries to use function that did not +# exist in the defined version. These risk breaking builds +conf.set('GLIB_VERSION_MAX_ALLOWED', 'GLIB_VERSION_' + glib_version_arr[0] + '_' + glib_version_arr[1])
glusterfs_version = '3.4.1' glusterfs_dep = dependency('glusterfs-api', version: '>=' + glusterfs_version, required: get_option('glusterfs'))
Otherwise looks good. Pavel

On 5/3/21 10:17 AM, Pavel Hrdina wrote:
On Fri, Apr 30, 2021 at 07:18:49AM +0200, Michal Privoznik wrote:
So far we have three places where glib version is recorded: meson.build and then in config.h. The latter is so well hidden that it's easy to miss when bumping minimal glib version in the former. With a bit of python^Wmeson string magic GLIB_VERSION_MIN_REQUIRED and GLIB_VERSION_MAX_ALLOWED macros can be defined to match glib_version from meson.build.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- config.h | 10 ---------- meson.build | 7 +++++++ 2 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/config.h b/config.h index ca6720f37d..0eacfd139d 100644 --- a/config.h +++ b/config.h @@ -51,13 +51,3 @@ #else # error You either need at least GCC 4.8 or Clang 3.4 or XCode Clang 5.1 to compile libvirt #endif - -/* Ask for warnings for anything that was marked deprecated in - * the defined version, or before. It is a candidate for rewrite. - */ -#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_48 - -/* Ask for warnings if code tries to use function that did not - * exist in the defined version. These risk breaking builds - */ -#define GLIB_VERSION_MAX_ALLOWED GLIB_VERSION_2_48 diff --git a/meson.build b/meson.build index 951da67896..597e5d8a13 100644 --- a/meson.build +++ b/meson.build @@ -949,6 +949,13 @@ endif glib_dep = declare_dependency( dependencies: [ glib_dep, gobject_dep, gio_dep ], ) +glib_version_arr=glib_version.split('.')
s/=/ = /
In the spirit of removing duplicated code how about having another variable:
glib_version_str = 'GLIB_VERSION_@0@_@1@'.format(glib_version_arr[0], glib_version_arr[1])
and using it in the following code?
Sounds good. I was happy that it worked and did not think that much about it. Let me send v2. Michal
participants (3)
-
Michal Privoznik
-
Michal Prívozník
-
Pavel Hrdina