diff mbox series

[v3,5/6] plugin: getting qemu_plugin_get_hwaddr only expose one function prototype

Message ID 20201001163429.1348-6-luoyonggang@gmail.com
State Superseded
Headers show
Series Enable plugin support on msys2/mingw | expand

Commit Message

罗勇刚(Yonggang Luo) Oct. 1, 2020, 4:34 p.m. UTC
This is used for counting how much function are export to qemu plugin.

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 plugins/api.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Comments

Alex Bennée Oct. 5, 2020, 10:48 a.m. UTC | #1
Yonggang Luo <luoyonggang@gmail.com> writes:

> This is used for counting how much function are export to qemu plugin.

>

> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>

> ---

>  plugins/api.c | 8 +++-----

>  1 file changed, 3 insertions(+), 5 deletions(-)

>

> diff --git a/plugins/api.c b/plugins/api.c

> index f16922ca8b..d325084385 100644

> --- a/plugins/api.c

> +++ b/plugins/api.c

> @@ -252,10 +252,12 @@ bool qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info)

>  

>  #ifdef CONFIG_SOFTMMU

>  static __thread struct qemu_plugin_hwaddr hwaddr_info;

> +#endif

>  

>  struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,

>                                                    uint64_t vaddr)

>  {

> +#ifdef CONFIG_SOFTMMU

>      CPUState *cpu = current_cpu;

>      unsigned int mmu_idx = info >> TRACE_MEM_MMU_SHIFT;

>      hwaddr_info.is_store = info & TRACE_MEM_ST;

> @@ -267,14 +269,10 @@ struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,

>      }

>  

>      return &hwaddr_info;

> -}

>  #else

> -struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,

> -                                                  uint64_t vaddr)

> -{

>      return NULL;

> -}

>  #endif

> +}


Hmm I'm not sure about this, surely you want the plugin system to
complain early if your plugin is going to use a function that is
incorrect for the mode you are running in?

Although we do currently unconditionally export the syscall functions
and arguably they should be CONFIG_USER only as well.

>  

>  bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr)

>  {



-- 
Alex Bennée
罗勇刚(Yonggang Luo) Oct. 5, 2020, 3:34 p.m. UTC | #2
On Mon, Oct 5, 2020 at 6:48 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>

>

> Yonggang Luo <luoyonggang@gmail.com> writes:

>

> > This is used for counting how much function are export to qemu plugin.

> >

> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>

> > ---

> >  plugins/api.c | 8 +++-----

> >  1 file changed, 3 insertions(+), 5 deletions(-)

> >

> > diff --git a/plugins/api.c b/plugins/api.c

> > index f16922ca8b..d325084385 100644

> > --- a/plugins/api.c

> > +++ b/plugins/api.c

> > @@ -252,10 +252,12 @@ bool

qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info)
> >

> >  #ifdef CONFIG_SOFTMMU

> >  static __thread struct qemu_plugin_hwaddr hwaddr_info;

> > +#endif

> >

> >  struct qemu_plugin_hwaddr

*qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
> >                                                    uint64_t vaddr)

> >  {

> > +#ifdef CONFIG_SOFTMMU

> >      CPUState *cpu = current_cpu;

> >      unsigned int mmu_idx = info >> TRACE_MEM_MMU_SHIFT;

> >      hwaddr_info.is_store = info & TRACE_MEM_ST;

> > @@ -267,14 +269,10 @@ struct qemu_plugin_hwaddr

*qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
> >      }

> >

> >      return &hwaddr_info;

> > -}

> >  #else

> > -struct qemu_plugin_hwaddr

*qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
> > -                                                  uint64_t vaddr)

> > -{

> >      return NULL;

> > -}

> >  #endif

> > +}

>

> Hmm I'm not sure about this, surely you want the plugin system to

> complain early if your plugin is going to use a function that is

> incorrect for the mode you are running in?

I merged these two function for couting how much function are exported, so
getting the code easier to review, otherwise
 function qemu_plugin_get_hwaddr   would be exported twice.
>

> Although we do currently unconditionally export the syscall functions

> and arguably they should be CONFIG_USER only as well.

>

> >

> >  bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr)

> >  {

>

>

> --

> Alex Bennée




--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo
<div dir="ltr"><br><br>On Mon, Oct 5, 2020 at 6:48 PM Alex Bennée &lt;<a href="mailto:alex.bennee@linaro.org">alex.bennee@linaro.org</a>&gt; wrote:<br>&gt;<br>&gt;<br>&gt; Yonggang Luo &lt;<a href="mailto:luoyonggang@gmail.com">luoyonggang@gmail.com</a>&gt; writes:<br>&gt;<br>&gt; &gt; This is used for counting how much function are export to qemu plugin.<br>&gt; &gt;<br>&gt; &gt; Signed-off-by: Yonggang Luo &lt;<a href="mailto:luoyonggang@gmail.com">luoyonggang@gmail.com</a>&gt;<br>&gt; &gt; ---<br>&gt; &gt;  plugins/api.c | 8 +++-----<br>&gt; &gt;  1 file changed, 3 insertions(+), 5 deletions(-)<br>&gt; &gt;<br>&gt; &gt; diff --git a/plugins/api.c b/plugins/api.c<br>&gt; &gt; index f16922ca8b..d325084385 100644<br>&gt; &gt; --- a/plugins/api.c<br>&gt; &gt; +++ b/plugins/api.c<br>&gt; &gt; @@ -252,10 +252,12 @@ bool qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info)<br>&gt; &gt; <br>&gt; &gt;  #ifdef CONFIG_SOFTMMU<br>&gt; &gt;  static __thread struct qemu_plugin_hwaddr hwaddr_info;<br>&gt; &gt; +#endif<br>&gt; &gt; <br>&gt; &gt;  struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,<br>&gt; &gt;                                                    uint64_t vaddr)<br>&gt; &gt;  {<br>&gt; &gt; +#ifdef CONFIG_SOFTMMU<br>&gt; &gt;      CPUState *cpu = current_cpu;<br>&gt; &gt;      unsigned int mmu_idx = info &gt;&gt; TRACE_MEM_MMU_SHIFT;<br>&gt; &gt;      hwaddr_info.is_store = info &amp; TRACE_MEM_ST;<br>&gt; &gt; @@ -267,14 +269,10 @@ struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,<br>&gt; &gt;      }<br>&gt; &gt; <br>&gt; &gt;      return &amp;hwaddr_info;<br>&gt; &gt; -}<br>&gt; &gt;  #else<br>&gt; &gt; -struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,<br>&gt; &gt; -                                                  uint64_t vaddr)<br>&gt; &gt; -{<br>&gt; &gt;      return NULL;<br>&gt; &gt; -}<br>&gt; &gt;  #endif<br>&gt; &gt; +}<br>&gt;<br>&gt; Hmm I&#39;m not sure about this, surely you want the plugin system to<br>&gt; complain early if your plugin is going to use a function that is<br>&gt; incorrect for the mode you are running in?<div>I merged these two function for couting how much function are exported, so getting the code easier to review, otherwise</div><div> function qemu_plugin_get_hwaddr   would be exported twice.<br>&gt;<br>&gt; Although we do currently unconditionally export the syscall functions<br>&gt; and arguably they should be CONFIG_USER only as well.<br>&gt;<br>&gt; &gt; <br>&gt; &gt;  bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr)<br>&gt; &gt;  {<br>&gt;<br>&gt;<br>&gt; --<br>&gt; Alex Bennée<br><br><br><br>--<br>         此致<br>礼<br>罗勇刚<br>Yours<br>    sincerely,<br>Yonggang Luo</div></div>
Alex Bennée Oct. 5, 2020, 3:46 p.m. UTC | #3
罗勇刚(Yonggang Luo) <luoyonggang@gmail.com> writes:

> On Mon, Oct 5, 2020 at 6:48 PM Alex Bennée <alex.bennee@linaro.org> wrote:

>>

>>

>> Yonggang Luo <luoyonggang@gmail.com> writes:

>>

>> > This is used for counting how much function are export to qemu plugin.

>> >

>> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>

>> > ---

>> >  plugins/api.c | 8 +++-----

>> >  1 file changed, 3 insertions(+), 5 deletions(-)

>> >

>> > diff --git a/plugins/api.c b/plugins/api.c

>> > index f16922ca8b..d325084385 100644

>> > --- a/plugins/api.c

>> > +++ b/plugins/api.c

>> > @@ -252,10 +252,12 @@ bool

> qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info)

>> >

>> >  #ifdef CONFIG_SOFTMMU

>> >  static __thread struct qemu_plugin_hwaddr hwaddr_info;

>> > +#endif

>> >

>> >  struct qemu_plugin_hwaddr

> *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,

>> >                                                    uint64_t vaddr)

>> >  {

>> > +#ifdef CONFIG_SOFTMMU

>> >      CPUState *cpu = current_cpu;

>> >      unsigned int mmu_idx = info >> TRACE_MEM_MMU_SHIFT;

>> >      hwaddr_info.is_store = info & TRACE_MEM_ST;

>> > @@ -267,14 +269,10 @@ struct qemu_plugin_hwaddr

> *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,

>> >      }

>> >

>> >      return &hwaddr_info;

>> > -}

>> >  #else

>> > -struct qemu_plugin_hwaddr

> *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,

>> > -                                                  uint64_t vaddr)

>> > -{

>> >      return NULL;

>> > -}

>> >  #endif

>> > +}

>>

>> Hmm I'm not sure about this, surely you want the plugin system to

>> complain early if your plugin is going to use a function that is

>> incorrect for the mode you are running in?

> I merged these two function for couting how much function are exported, so

> getting the code easier to review, otherwise

>  function qemu_plugin_get_hwaddr   would be exported twice.


Ahh I see now..

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


-- 
Alex Bennée
diff mbox series

Patch

diff --git a/plugins/api.c b/plugins/api.c
index f16922ca8b..d325084385 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -252,10 +252,12 @@  bool qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info)
 
 #ifdef CONFIG_SOFTMMU
 static __thread struct qemu_plugin_hwaddr hwaddr_info;
+#endif
 
 struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
                                                   uint64_t vaddr)
 {
+#ifdef CONFIG_SOFTMMU
     CPUState *cpu = current_cpu;
     unsigned int mmu_idx = info >> TRACE_MEM_MMU_SHIFT;
     hwaddr_info.is_store = info & TRACE_MEM_ST;
@@ -267,14 +269,10 @@  struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
     }
 
     return &hwaddr_info;
-}
 #else
-struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
-                                                  uint64_t vaddr)
-{
     return NULL;
-}
 #endif
+}
 
 bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr)
 {