Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/tcg/tcg-all.c    | 2 +-
 include/sysemu/accel.h | 2 +-
 vl.c                   | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index dba9931..e327d90 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -28,7 +28,7 @@
 #include "sysemu/sysemu.h"
 #include "qom/object.h"
 
-int tcg_tb_size;
+unsigned long tcg_tb_size;
 static bool tcg_allowed = true;
 
 static int tcg_init(MachineState *ms)
diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
index ecc5c84..5a632ce 100644
--- a/include/sysemu/accel.h
+++ b/include/sysemu/accel.h
@@ -63,7 +63,7 @@ typedef struct AccelClass {
 #define ACCEL_GET_CLASS(obj) \
     OBJECT_GET_CLASS(AccelClass, (obj), TYPE_ACCEL)
 
-extern int tcg_tb_size;
+extern unsigned long tcg_tb_size;
 
 void configure_accelerator(MachineState *ms);
 /* Register accelerator specific global properties */
diff --git a/vl.c b/vl.c
index 36ff3f4..611ddfe 100644
--- a/vl.c
+++ b/vl.c
@@ -3933,9 +3933,9 @@ int main(int argc, char **argv, char **envp)
                 configure_rtc(opts);
                 break;
             case QEMU_OPTION_tb_size:
-                tcg_tb_size = strtol(optarg, NULL, 0);
-                if (tcg_tb_size < 0) {
-                    tcg_tb_size = 0;
+                if (qemu_strtoul(optarg, NULL, 0, &tcg_tb_size) < 0) {
+                    error_report("Invalid argument to -tb-size");
+                    exit(1);
                 }
                 break;
             case QEMU_OPTION_icount:
-- 
1.8.3.1

On 07/03/2017 09:34 AM, Paolo Bonzini wrote:
> -extern int tcg_tb_size;
> +extern unsigned long tcg_tb_size;

size_t would be more natural.

I don't think we support any hosts for which sizeof(size_t) != sizeof(unsigned 
long), but perhaps

	unsigned lomg tmp;
	if (qemu_strtoul(optarg, NULL, 0, &tmp) < 0
             || tmp != (size_t)tmp) {
	  error_report("Invalid argument to -tb-size");
	  exit(1);
	}
	tcg_tb_size = tmp;

where I'd expect the extra comparison to be optimized away.

But I'm not overly concerned either way, so

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

On 03/07/2017 21:46, Richard Henderson wrote:
> On 07/03/2017 09:34 AM, Paolo Bonzini wrote:
>> -extern int tcg_tb_size;
>> +extern unsigned long tcg_tb_size;
> 
> size_t would be more natural.
> 
> I don't think we support any hosts for which sizeof(size_t) !=
> sizeof(unsigned long), but perhaps

There's Win64...  Another place where to do the range check
could be tcg_exec_init.  That's where the actual bug lies.

The previous code's error handling was even worse, since strtol's output
was completely unchecked.  tcg_exec_init can be fixed later.

Paolo

>     unsigned lomg tmp;
>     if (qemu_strtoul(optarg, NULL, 0, &tmp) < 0
>             || tmp != (size_t)tmp) {
>       error_report("Invalid argument to -tb-size");
>       exit(1);
>     }
>     tcg_tb_size = tmp;
> 
> where I'd expect the extra comparison to be optimized away.
> 
> But I'm not overly concerned either way, so
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> 
> 
> r~

On Mon, Jul 03, 2017 at 12:46:06PM -0700, Richard Henderson wrote:
> On 07/03/2017 09:34 AM, Paolo Bonzini wrote:
> > -extern int tcg_tb_size;
> > +extern unsigned long tcg_tb_size;
> 
> size_t would be more natural.
> 
> I don't think we support any hosts for which sizeof(size_t) !=
> sizeof(unsigned long), but perhaps
> 
> 	unsigned lomg tmp;
> 	if (qemu_strtoul(optarg, NULL, 0, &tmp) < 0
>             || tmp != (size_t)tmp) {
> 	  error_report("Invalid argument to -tb-size");
> 	  exit(1);
> 	}
> 	tcg_tb_size = tmp;
> 
> where I'd expect the extra comparison to be optimized away.

If we wanted to use size_t, then I'd suggest we added a qemu_strtoXXX
variant that directly returned a size_t, and did suitabled bounds
checking

> But I'm not overly concerned either way, so

Agreed, doesn't need fixing now

> Reviewed-by: Richard Henderson <rth@twiddle.net>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|