Re: [Qemu-devel] [PATCH 04/22] vl: convert -tb-size to qemu_strtoul

2017-07-04 Thread Paolo Bonzini


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, ) < 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 
> 
> 
> r~



Re: [Qemu-devel] [PATCH 04/22] vl: convert -tb-size to qemu_strtoul

2017-07-04 Thread Daniel P. Berrange
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, ) < 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 

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 :|



Re: [Qemu-devel] [PATCH 04/22] vl: convert -tb-size to qemu_strtoul

2017-07-03 Thread Richard Henderson

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, ) < 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 


r~