[libvirt] [PATCHv2 1/2] command: improve allocation failure reporting

[Jiri Denemark]

> * 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