| Message ID | 20201001163429.1348-5-luoyonggang@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Enable plugin support on msys2/mingw | expand |
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
Hi, I split this out just for easier review, so the lines changed in api.c and core.c equales to the number of function exported, anyway On Mon, Oct 5, 2020 at 6:44 PM Alex Bennée <alex.bennee@linaro.org> wrote: > > > 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 -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo <div dir="ltr">Hi, I split this out just for easier review, so the lines changed in api.c and core.c<div>equales to the number of function exported, anyway<br><br>On Mon, Oct 5, 2020 at 6:44 PM Alex Bennée <<a href="mailto:alex.bennee@linaro.org">alex.bennee@linaro.org</a>> wrote:<br>><br>><br>> Yonggang Luo <<a href="mailto:luoyonggang@gmail.com">luoyonggang@gmail.com</a>> writes:<br>><br>> > This is used to distinguish from the qemu and plugin in<br>> > header qemu-plugin.h<br>> ><br>> > Signed-off-by: Yonggang Luo <<a href="mailto:luoyonggang@gmail.com">luoyonggang@gmail.com</a>><br>> > ---<br>> > plugins/api.c | 1 +<br>> > plugins/core.c | 1 +<br>> > 2 files changed, 2 insertions(+)<br>> ><br>> > diff --git a/plugins/api.c b/plugins/api.c<br>> > index bbdc5a4eb4..f16922ca8b 100644<br>> > --- a/plugins/api.c<br>> > +++ b/plugins/api.c<br>> > @@ -35,6 +35,7 @@<br>> > */<br>> > <br>> > #include "qemu/osdep.h"<br>> > +#define QEMU_PLUGIN_API_IMPLEMENTATION<br>> > #include "qemu/plugin.h"<br>><br>> This doesn't do anything in this patch. You should split the special<br>> handling in the plugin header and combine it in this patch. Also I'm not<br>> quite sure of the logic you are trying to achieve later on:<br>><br>> #if !defined(QEMU_PLUGIN_API_IMPLEMENTATION)<br>> #if defined(QEMU_PLUGIN_IMPLEMENTATION)<br>> #define QEMU_PLUGIN_EXTERN<br>> #else<br>> #define QEMU_PLUGIN_EXTERN extern<br>> #endif<br>><br>> It seems to me you could get away with only one symbol - ideally just<br>> defined in plugins/api.c so you don't need to churn the plugins with<br>> changes, e.g.<br>><br>> #ifdef QEMU_PLUGIN_API_IMPLEMENTATION<br>> #define QEMU_PLUGIN_EXTERN<br>> #else<br>> #define QEMU_PLUGIN_EXTERN extern<br>> #endif<br>><br>> But I'd still like to have a better answer as to why we need the extern?<br>> Is this a limitation of the mingw compiler or some windows special?<br>><br>> > #include "cpu.h"<br>> > #include "sysemu/sysemu.h"<br>> > diff --git a/plugins/core.c b/plugins/core.c<br>> > index 51bfc94787..7a79ea4179 100644<br>> > --- a/plugins/core.c<br>> > +++ b/plugins/core.c<br>> > @@ -12,6 +12,7 @@<br>> > * SPDX-License-Identifier: GPL-2.0-or-later<br>> > */<br>> > #include "qemu/osdep.h"<br>> > +#define QEMU_PLUGIN_API_IMPLEMENTATION<br>> > #include "qemu/error-report.h"<br>> > #include "qemu/config-file.h"<br>> > #include "qapi/error.h"<br>><br>> I don't think we include qemu/plugin.h from this file although it does<br>> raise the question of what happens when other parts of QEMU include the<br>> internal qemu/plugins.h header.<br>><br>> --<br>> Alex Bennée<br><br><br><br>--<br> 此致<br>礼<br>罗勇刚<br>Yours<br> sincerely,<br>Yonggang Luo</div></div>
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" #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"
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(+)