9 a.m.
This series touches virfirewall.c, the last place where a no_memory
label can be found.
The series:
- Gets rid of setting and checking for ENOMEM as a firewall's error;
- Use g_auto / g_autofree in a few different places;
- Adapt ADD_ARG() macro as VIR_RESIZE_N() just aborts in case of OOM;
- Remove no_memory mention from the hacking guide;
Changes since v1:
https://www.redhat.com/archives/libvir-list/2020-January/msg00018.html
- Daniel suggested to get rid of ADD_ARG(). Instead of doing so, I've
adapted the macro (as it does more than just a realloc) and, by doing
so, got rid of the no_memory labels;
- Dropped:
- util: Rename 'no_memory' label to 'cleanup'
- util: Add ADD_ARG_RETURN_ON_ERROR
- util: Add ADD_ARG_RETURN_VALUE_ON_ERROR
- Added:
- util: Adapt ADD_ARG() macro
Fabiano Fidêncio (5):
util: Don't set/check for ENOMEM as a firewall error
util: Use g_auto/g_autofree in virFirewallAddRuleFullV()
util: Use g_auto in virFirewallStartTransaction()
util: Adapt ADD_ARG() macro
docs: Remove mention to no_memory label
docs/hacking.html.in | 1 -
src/util/virfirewall.c | 65 +++++++++++-------------------------------
2 files changed, 16 insertions(+), 50 deletions(-)
--
2.24.1
9 a.m.
New subject: [libvirt] [PATCH v2 1/5] util: Don't set/check for ENOMEM as a firewall error
As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's not set nor check for ENOMEM firewall's error and
simplify the code whenever it's possible.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/util/virfirewall.c | 30 ++++++------------------------
1 file changed, 6 insertions(+), 24 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index ee72b579e4..6f7b5306e5 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -391,7 +391,6 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
return rule;
no_memory:
- firewall->err = ENOMEM;
virFirewallRuleFree(rule);
return NULL;
}
@@ -492,10 +491,8 @@ void virFirewallRuleAddArg(virFirewallPtr firewall,
ADD_ARG(rule, arg);
- return;
-
no_memory:
- firewall->err = ENOMEM;
+ return;
}
@@ -514,10 +511,8 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
ADD_ARG(rule, arg);
- return;
-
no_memory:
- firewall->err = ENOMEM;
+ return;
}
@@ -532,10 +527,8 @@ void virFirewallRuleAddArgSet(virFirewallPtr firewall,
args++;
}
- return;
-
no_memory:
- firewall->err = ENOMEM;
+ return;
}
@@ -553,12 +546,7 @@ void virFirewallRuleAddArgList(virFirewallPtr firewall,
while ((str = va_arg(list, char *)) != NULL)
ADD_ARG(rule, str);
- va_end(list);
-
- return;
-
no_memory:
- firewall->err = ENOMEM;
va_end(list);
}
@@ -591,15 +579,13 @@ void virFirewallStartTransaction(virFirewallPtr firewall,
VIR_FIREWALL_RETURN_IF_ERROR(firewall);
- if (!(group = virFirewallGroupNew())) {
- firewall->err = ENOMEM;
+ if (!(group = virFirewallGroupNew()))
return;
- }
+
group->actionFlags = flags;
if (VIR_EXPAND_N(firewall->groups,
firewall->ngroups, 1) < 0) {
- firewall->err = ENOMEM;
virFirewallGroupFree(group);
return;
}
@@ -747,10 +733,6 @@ virFirewallApplyRule(virFirewallPtr firewall,
if (rule->queryCB(firewall, rule->layer, (const char *const *)lines,
rule->queryOpaque) < 0)
return -1;
- if (firewall->err == ENOMEM) {
- virReportOOMError();
- return -1;
- }
if (firewall->err) {
virReportSystemError(firewall->err, "%s",
_("Unable to create rule"));
@@ -818,7 +800,7 @@ virFirewallApply(virFirewallPtr firewall)
_("Failed to initialize a valid firewall backend"));
goto cleanup;
}
- if (!firewall || firewall->err == ENOMEM) {
+ if (!firewall) {
virReportOOMError();
goto cleanup;
}
--
2.24.1
9 a.m.
New subject: [libvirt] [PATCH v2 2/5] util: Use g_auto/g_autofree in virFirewallAddRuleFullV()
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/util/virfirewall.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 6f7b5306e5..2177617ecf 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -257,6 +257,7 @@ virFirewallRuleFree(virFirewallRulePtr rule)
VIR_FREE(rule);
}
+G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virFirewallRulePtr, virFirewallRuleFree, NULL);
static void
virFirewallGroupFree(virFirewallGroupPtr group)
@@ -335,8 +336,8 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
va_list args)
{
virFirewallGroupPtr group;
- virFirewallRulePtr rule;
- char *str;
+ g_auto(virFirewallRulePtr) rule = NULL;
+ g_autofree char *str = NULL;
VIR_FIREWALL_RETURN_NULL_IF_ERROR(firewall);
@@ -348,7 +349,7 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
if (VIR_ALLOC(rule) < 0)
- goto no_memory;
+ return NULL;
rule->layer = layer;
rule->queryCB = cb;
@@ -379,19 +380,18 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
if (VIR_APPEND_ELEMENT_COPY(group->rollback,
group->nrollback,
rule) < 0)
- goto no_memory;
+ return NULL;
} else {
if (VIR_APPEND_ELEMENT_COPY(group->action,
group->naction,
rule) < 0)
- goto no_memory;
+ return NULL;
}
- return rule;
+ return g_steal_pointer(&rule);
no_memory:
- virFirewallRuleFree(rule);
return NULL;
}
--
2.24.1
9 a.m.
New subject: [libvirt] [PATCH v2 3/5] util: Use g_auto in virFirewallStartTransaction()
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/util/virfirewall.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 2177617ecf..564e2fe0be 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -278,6 +278,7 @@ virFirewallGroupFree(virFirewallGroupPtr group)
VIR_FREE(group);
}
+G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virFirewallGroupPtr, virFirewallGroupFree, NULL);
/**
* virFirewallFree:
@@ -575,7 +576,7 @@ size_t virFirewallRuleGetArgCount(virFirewallRulePtr rule)
void virFirewallStartTransaction(virFirewallPtr firewall,
unsigned int flags)
{
- virFirewallGroupPtr group;
+ g_auto(virFirewallGroupPtr) group = NULL;
VIR_FIREWALL_RETURN_IF_ERROR(firewall);
@@ -586,10 +587,9 @@ void virFirewallStartTransaction(virFirewallPtr firewall,
if (VIR_EXPAND_N(firewall->groups,
firewall->ngroups, 1) < 0) {
- virFirewallGroupFree(group);
return;
}
- firewall->groups[firewall->ngroups - 1] = group;
+ firewall->groups[firewall->ngroups - 1] = g_steal_pointer(&group);
firewall->currentGroup = firewall->ngroups - 1;
}
--
2.24.1
9 a.m.
New subject: [libvirt] [PATCH v2 4/5] util: Adapt ADD_ARG() macro
As VIR_RESIZE_N() macro already aborts in case of OOM, there's no reason
to check for its output in the ADD_ARG() macro.
By doing this, we can simply get rid of a all no_memory labels spread in
the virfirewall.c file.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/util/virfirewall.c | 21 +++------------------
1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 564e2fe0be..7c8040880c 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -320,11 +320,9 @@ void virFirewallFree(virFirewallPtr firewall)
#define ADD_ARG(rule, str) \
do { \
- if (VIR_RESIZE_N(rule->args, \
- rule->argsAlloc, \
- rule->argsLen, 1) < 0) \
- goto no_memory; \
- \
+ ignore_value(VIR_RESIZE_N(rule->args, \
+ rule->argsAlloc, \
+ rule->argsLen, 1)); \
rule->args[rule->argsLen++] = g_strdup(str); \
} while (0)
@@ -391,9 +389,6 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
return g_steal_pointer(&rule);
-
- no_memory:
- return NULL;
}
@@ -491,9 +486,6 @@ void virFirewallRuleAddArg(virFirewallPtr firewall,
VIR_FIREWALL_RULE_RETURN_IF_ERROR(firewall, rule);
ADD_ARG(rule, arg);
-
- no_memory:
- return;
}
@@ -511,9 +503,6 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
va_end(list);
ADD_ARG(rule, arg);
-
- no_memory:
- return;
}
@@ -527,9 +516,6 @@ void virFirewallRuleAddArgSet(virFirewallPtr firewall,
ADD_ARG(rule, *args);
args++;
}
-
- no_memory:
- return;
}
@@ -547,7 +533,6 @@ void virFirewallRuleAddArgList(virFirewallPtr firewall,
while ((str = va_arg(list, char *)) != NULL)
ADD_ARG(rule, str);
- no_memory:
va_end(list);
}
--
2.24.1
9 a.m.
New subject: [libvirt] [PATCH v2 5/5] docs: Remove mention to no_memory label
no_memory labels have been entirely removed from libvirt code base.
Knowing that, let's also remove any mention to the label from our
hacking guide.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
docs/hacking.html.in | 1 -
1 file changed, 1 deletion(-)
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 74aba5d46b..90bd0ddc81 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -1491,7 +1491,6 @@ BAD:
<pre>
error: A path only taken upon return with an error code
cleanup: A path taken upon return with success code + optional error
- no_memory: A path only taken upon return with an OOM error code
retry: If needing to jump upwards (e.g., retry on EINTR)
</pre>
--
2.24.1
1785
days inactive
1785
days old
5 comments
1 participants
participants (1)
-
Fabiano Fidêncio