Re: [libvirt] [PATCHv2 1/2] command: improve allocation failure reporting
* src/util/command.c (virCommandAddEnvString): Remove duplicate
code.
(virCommandToString, virCommandRun, virCommandRunAsync)
(virCommandWait): Report NULL command as ENOMEM, not invalid
usage.
Reported by Jiri Denemark.
---
New patch. Fixes the root cause, so that patch 2 can
be smaller...
Yeah, makes sense. I didn't realize the expected behavior was not to require
error checking even after virCommandNew*
diff --git a/src/util/command.c b/src/util/command.c
index 38d462b..c0520ec 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -292,9 +292,6 @@ virCommandAddEnvPair(virCommandPtr cmd, const char *name, const char
*value)
void
virCommandAddEnvString(virCommandPtr cmd, const char *str)
{
- if (!cmd || cmd->has_error)
- return;
-
char *env;
if (!cmd || cmd->has_error)
Eh.
@@ -710,13 +707,13 @@ virCommandToString(virCommandPtr cmd)
/* Cannot assume virCommandRun will be called; so report the error
* now. If virCommandRun is called, it will report the same error. */
- if (!cmd || cmd->has_error == -1) {
- virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("invalid use of command API"));
+ if (!cmd ||cmd->has_error == ENOMEM) {
+ virReportOOMError();
return NULL;
}
- if (cmd->has_error == ENOMEM) {
- virReportOOMError();
+ if (cmd->has_error) {
+ virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("invalid use of command API"));
return NULL;
}
I checked has_error is either 0, -1, or ENOMEM so we are not hiding any error
here.
ACK
Jirka