Re: [PATCH v3 4/6] plugin: define QEMU_PLUGIN_API_IMPLEMENTATION first

[Alex Bennée]

Yonggang Luo <luoyonggang@gmail.com> writes:

> This is used to distinguish from the qemu and plugin in
> header qemu-plugin.h
>
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  plugins/api.c  | 1 +
>  plugins/core.c | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/plugins/api.c b/plugins/api.c
> index bbdc5a4eb4..f16922ca8b 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -35,6 +35,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#define QEMU_PLUGIN_API_IMPLEMENTATION
>  #include "qemu/plugin.h"

This doesn't do anything in this patch. You should split the special
handling in the plugin header and combine it in this patch. Also I'm not
quite sure of the logic you are trying to achieve later on:

  #if !defined(QEMU_PLUGIN_API_IMPLEMENTATION)
  #if defined(QEMU_PLUGIN_IMPLEMENTATION)
  #define QEMU_PLUGIN_EXTERN
  #else
  #define QEMU_PLUGIN_EXTERN extern
  #endif

It seems to me you could get away with only one symbol - ideally just
defined in plugins/api.c so you don't need to churn the plugins with
changes, e.g.

  #ifdef QEMU_PLUGIN_API_IMPLEMENTATION
  #define QEMU_PLUGIN_EXTERN
  #else
  #define QEMU_PLUGIN_EXTERN extern
  #endif

But I'd still like to have a better answer as to why we need the extern?
Is this a limitation of the mingw compiler or some windows special?

>  #include "cpu.h"
>  #include "sysemu/sysemu.h"
> diff --git a/plugins/core.c b/plugins/core.c
> index 51bfc94787..7a79ea4179 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -12,6 +12,7 @@
>   * SPDX-License-Identifier: GPL-2.0-or-later
>   */
>  #include "qemu/osdep.h"
> +#define QEMU_PLUGIN_API_IMPLEMENTATION
>  #include "qemu/error-report.h"
>  #include "qemu/config-file.h"
>  #include "qapi/error.h"

I don't think we include qemu/plugin.h from this file although it does
raise the question of what happens when other parts of QEMU include the
internal qemu/plugins.h header.

-- 
Alex Bennée