[Qemu-devel] [PATCH] oslib-posix: Fix compiler warning and some data types

[Qemu-devel] [PATCH] oslib-posix: Fix compiler warning and some data types

[Stefan Weil]

gcc warning:

/qemu/util/oslib-posix.c:304:11: error:
 variable ‘addr’ might be clobbered by ‘longjmp’ or ‘vfork’
 [-Werror=clobbered]

Fix also some related data types:

numpages, hpagesize are used as pointer offset.
Always use size_t for them and also for the derived
numpages_per_thread and size_per_thread.

Avoid a type cast by declaring addr volatile.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---

v2: Fix more data types (partially as discussed with Richard)

v3: Keep volatile type cast (suggested by Philippe and Paolo)

Regards
Stefan

 util/oslib-posix.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 80086c549f..382bd4a231 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -59,8 +59,8 @@
 
 struct MemsetThread {
     char *addr;
-    uint64_t numpages;
-    uint64_t hpagesize;
+    size_t numpages;
+    size_t hpagesize;
     QemuThread pgthread;
     sigjmp_buf env;
 };
@@ -301,11 +301,7 @@ static void sigbus_handler(int signal)
 static void *do_touch_pages(void *arg)
 {
     MemsetThread *memset_args = (MemsetThread *)arg;
-    char *addr = memset_args->addr;
-    uint64_t numpages = memset_args->numpages;
-    uint64_t hpagesize = memset_args->hpagesize;
     sigset_t set, oldset;
-    int i = 0;
 
     /* unblock SIGBUS */
     sigemptyset(&set);
@@ -315,6 +311,10 @@ static void *do_touch_pages(void *arg)
     if (sigsetjmp(memset_args->env, 1)) {
         memset_thread_failed = true;
     } else {
+        char *addr = memset_args->addr;
+        size_t numpages = memset_args->numpages;
+        size_t hpagesize = memset_args->hpagesize;
+        size_t i;
         for (i = 0; i < numpages; i++) {
             /*
              * Read & write back the same value, so we don't
@@ -351,7 +351,8 @@ static inline int get_memset_num_threads(int smp_cpus)
 static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
                             int smp_cpus)
 {
-    uint64_t numpages_per_thread, size_per_thread;
+    size_t numpages_per_thread;
+    size_t size_per_thread;
     char *addr = area;
     int i = 0;
 
-- 
2.11.0

Re: [Qemu-devel] [PATCH] oslib-posix: Fix compiler warning and some data types

[Stefan Weil]

Sorry, Philippe, I did not notice that "git send-email"
did not add you to the addressee list.

And I forgot to add "v3" to the subject line.

I‌ should not send e-mails late in the night ...

Stefan


Am 16.10.2017 um 22:29 schrieb Stefan Weil:
> gcc warning:
> 
> /qemu/util/oslib-posix.c:304:11: error:
>  variable ‘addr’ might be clobbered by ‘longjmp’ or ‘vfork’
>  [-Werror=clobbered]
> 
> Fix also some related data types:
> 
> numpages, hpagesize are used as pointer offset.
> Always use size_t for them and also for the derived
> numpages_per_thread and size_per_thread.
> 
> Avoid a type cast by declaring addr volatile.
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> 
> v2: Fix more data types (partially as discussed with Richard)
> 
> v3: Keep volatile type cast (suggested by Philippe and Paolo)
> 
> Regards
> Stefan
> 
>  util/oslib-posix.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 80086c549f..382bd4a231 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -59,8 +59,8 @@
>  
>  struct MemsetThread {
>      char *addr;
> -    uint64_t numpages;
> -    uint64_t hpagesize;
> +    size_t numpages;
> +    size_t hpagesize;
>      QemuThread pgthread;
>      sigjmp_buf env;
>  };
> @@ -301,11 +301,7 @@ static void sigbus_handler(int signal)
>  static void *do_touch_pages(void *arg)
>  {
>      MemsetThread *memset_args = (MemsetThread *)arg;
> -    char *addr = memset_args->addr;
> -    uint64_t numpages = memset_args->numpages;
> -    uint64_t hpagesize = memset_args->hpagesize;
>      sigset_t set, oldset;
> -    int i = 0;
>  
>      /* unblock SIGBUS */
>      sigemptyset(&set);
> @@ -315,6 +311,10 @@ static void *do_touch_pages(void *arg)
>      if (sigsetjmp(memset_args->env, 1)) {
>          memset_thread_failed = true;
>      } else {
> +        char *addr = memset_args->addr;
> +        size_t numpages = memset_args->numpages;
> +        size_t hpagesize = memset_args->hpagesize;
> +        size_t i;
>          for (i = 0; i < numpages; i++) {
>              /*
>               * Read & write back the same value, so we don't
> @@ -351,7 +351,8 @@ static inline int get_memset_num_threads(int smp_cpus)
>  static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
>                              int smp_cpus)
>  {
> -    uint64_t numpages_per_thread, size_per_thread;
> +    size_t numpages_per_thread;
> +    size_t size_per_thread;
>      char *addr = area;
>      int i = 0;
>  
> 

Re: [Qemu-devel] [PATCH] oslib-posix: Fix compiler warning and some data types

[Stefan Hajnoczi]

On Mon, Oct 16, 2017 at 10:29:12PM +0200, Stefan Weil wrote:
> gcc warning:
> 
> /qemu/util/oslib-posix.c:304:11: error:
>  variable ‘addr’ might be clobbered by ‘longjmp’ or ‘vfork’
>  [-Werror=clobbered]
> 
> Fix also some related data types:
> 
> numpages, hpagesize are used as pointer offset.
> Always use size_t for them and also for the derived
> numpages_per_thread and size_per_thread.
> 
> Avoid a type cast by declaring addr volatile.

I dropped this outdated line when merging the patch.

> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> 
> v2: Fix more data types (partially as discussed with Richard)
> 
> v3: Keep volatile type cast (suggested by Philippe and Paolo)
> 
> Regards
> Stefan
> 
>  util/oslib-posix.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan