[PATCH 0/3] syntax-check: Introduce sc_prohibit_g_autofree_const rule
Stems out from discussion here: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/HYTV... Michal Prívozník (3): hyperv: Drop const for resourceType in hypervDomainAttachSerial() network: Drop const for forwardIf in pfAddNatFirewallRules() syntax-check: Introduce sc_prohibit_g_autofree_const rule build-aux/syntax-check.mk | 6 ++++++ src/hyperv/hyperv_driver.c | 2 +- src/network/network_pf.c | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) -- 2.52.0
From: Michal Privoznik <mprivozn@redhat.com> The 'resourceType' variable inside of hypervDomainAttachSerial() is declared as both g_autofree and const. This makes no sense. Since the variable is g_strdup_printf()-ed into a couple of lines later, it's not const. Drop that part of variable declaration. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/hyperv/hyperv_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 0c17ef16ec..8dd56f39dc 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -974,7 +974,7 @@ hypervDomainAttachSerial(virDomainPtr domain, virDomainChrDef *serial) Msvm_ResourceAllocationSettingData *entry = NULL; g_autoptr(GHashTable) serialResource = NULL; const char *connectionValue = NULL; - g_autofree const char *resourceType = NULL; + g_autofree char *resourceType = NULL; virUUIDFormat(domain->uuid, uuid_string); -- 2.52.0
From: Michal Privoznik <mprivozn@redhat.com> The 'forwardIf' variable inside of pfAddNatFirewallRules() is declared as both g_autofree and const. This makes no sense. Since the variable is g_strdup()-ed into right in the declaration, it's not const. Drop that part of variable declaration. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/network/network_pf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/network_pf.c b/src/network/network_pf.c index 34e2f60e82..724b82c054 100644 --- a/src/network/network_pf.c +++ b/src/network/network_pf.c @@ -165,7 +165,7 @@ pfAddNatFirewallRules(virNetworkDef *def, * block log on virbr0 */ int prefix = virNetworkIPDefPrefix(ipdef); - g_autofree const char *forwardIf = g_strdup(virNetworkDefForwardIf(def, 0)); + g_autofree char *forwardIf = g_strdup(virNetworkDefForwardIf(def, 0)); g_auto(virBuffer) pf_rules_buf = VIR_BUFFER_INITIALIZER; g_autoptr(virCommand) cmd = virCommandNew(PFCTL); g_autoptr(virCommand) flush_cmd = virCommandNew(PFCTL); -- 2.52.0
Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
The 'forwardIf' variable inside of pfAddNatFirewallRules() is declared as both g_autofree and const. This makes no sense. Since the variable is g_strdup()-ed into right in the declaration, it's not const. Drop that part of variable declaration.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/network/network_pf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/network/network_pf.c b/src/network/network_pf.c index 34e2f60e82..724b82c054 100644 --- a/src/network/network_pf.c +++ b/src/network/network_pf.c @@ -165,7 +165,7 @@ pfAddNatFirewallRules(virNetworkDef *def, * block log on virbr0 */ int prefix = virNetworkIPDefPrefix(ipdef); - g_autofree const char *forwardIf = g_strdup(virNetworkDefForwardIf(def, 0)); + g_autofree char *forwardIf = g_strdup(virNetworkDefForwardIf(def, 0)); g_auto(virBuffer) pf_rules_buf = VIR_BUFFER_INITIALIZER; g_autoptr(virCommand) cmd = virCommandNew(PFCTL); g_autoptr(virCommand) flush_cmd = virCommandNew(PFCTL); -- 2.52.0
Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
From: Michal Privoznik <mprivozn@redhat.com> The aim of this rule is to capture the following pattern: g_autofree const char *var = ...; The pattern is problematic, because it frees a const pointer. If written the old way, we'd get a compiler warning: warning: passing argument 1 of ‘free’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- build-aux/syntax-check.mk | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 4deaa70368..f605c9b0e3 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -389,6 +389,12 @@ sc_prohibit_unsigned_pid: halt='use signed type for pid values' \ $(_sc_search_regexp) +sc_prohibit_g_autofree_const: + @prohibit='\<g_auto(free|ptr)\>.* const.* \*[^*]' \ + halt='‘g_autofree’ discards ‘const’ qualifier from pointer target type' \ + $(_sc_search_regexp) + + # Many of the function names below came from this filter: # git grep -B2 '\<_('|grep -E '\.c- *[[:alpha:]_][[:alnum:]_]* ?\(.*[,;]$' \ # |sed 's/.*\.c- *//'|perl -pe 's/ ?\(.*//'|sort -u \ -- 2.52.0
On Fri, Jan 09, 2026 at 08:33:49 +0100, Michal Privoznik wrote:
Stems out from discussion here:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/HYTV...
Michal Prívozník (3): hyperv: Drop const for resourceType in hypervDomainAttachSerial() network: Drop const for forwardIf in pfAddNatFirewallRules() syntax-check: Introduce sc_prohibit_g_autofree_const rule
build-aux/syntax-check.mk | 6 ++++++ src/hyperv/hyperv_driver.c | 2 +- src/network/network_pf.c | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-)
The regexp looks a bit strange to me (probably because I haven't seen all the lines it has to match or ignore), but I guess it work as is and if it stops working, we can update it later :-) Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
participants (3)
-
Jiri Denemark -
Michal Privoznik -
Roman Bogorodskiy