Re: [PATCH v2 20/22] qapi: Inline and remove QERR_QGA_COMMAND_FAILED definition

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Address the comment added in commit 4629ed1e98
> ("qerror: Finally unused, clean up"), from 2015:
>
>   /*
>    * These macros will go away, please don't use
>    * in new code, and do not add new ones!
>    */
>
> Mechanical transformation using the following
> coccinelle semantic patch:
>
>     @match exists@
>     expression errp;
>     expression errmsg;
>     @@
>          error_setg(errp, QERR_QGA_COMMAND_FAILED, errmsg);
>
>     @script:python strformat depends on match@
>     errmsg << match.errmsg;
>     fixedfmt; // new var
>     @@
>     # Format skipping '"'.
>     fixedfmt = f'"Guest agent command failed, error was \'{errmsg[1:-1]}\'"'
>     coccinelle.fixedfmt = cocci.make_ident(fixedfmt)
>
>     @replace@
>     expression match.errp;
>     expression match.errmsg;
>     identifier strformat.fixedfmt;
>     @@
>     -    error_setg(errp, QERR_QGA_COMMAND_FAILED, errmsg);
>     +    error_setg(errp, fixedfmt);
>
> then manually removing the definition in include/qapi/qmp/qerror.h.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/qapi/qmp/qerror.h |  3 ---
>  qga/commands-win32.c      | 38 ++++++++++++++++++++------------------
>  qga/commands.c            |  7 ++++---
>  3 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
> index e094f13114..840831cc6a 100644
> --- a/include/qapi/qmp/qerror.h
> +++ b/include/qapi/qmp/qerror.h
> @@ -17,9 +17,6 @@
>   * add new ones!
>   */
>  
> -#define QERR_QGA_COMMAND_FAILED \
> -    "Guest agent command failed, error was '%s'"
> -
>  #define QERR_UNSUPPORTED \
>      "this feature or command is not currently supported"
>  
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 946dbafbb6..aa8c9770d4 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -245,7 +245,8 @@ int64_t qmp_guest_file_open(const char *path, const char 
> *mode, Error **errp)
>  
>  done:
>      if (gerr) {
> -        error_setg(errp, QERR_QGA_COMMAND_FAILED, gerr->message);
> +        error_setg(errp,
> +                   "Guest agent command failed, error was 'err -> messag'");

Oopsie :)

Two more below.

>          g_error_free(gerr);
>      }
>      g_free(w_path);
> @@ -279,8 +280,8 @@ static void acquire_privilege(const char *name, Error 
> **errp)
>          TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY, &token))
>      {
>          if (!LookupPrivilegeValue(NULL, name, &priv.Privileges[0].Luid)) {
> -            error_setg(errp, QERR_QGA_COMMAND_FAILED,
> -                       "no luid for requested privilege");
> +            error_setg(errp,
> +                       "Guest agent command failed, error was 'no luid for 
> requested privilege'");
>              goto out;
>          }
>  

I don't like this error message.  I'm going to pretend I didn't see it.

[...]