| Message ID | 1250171428-29308-24-git-send-email-lcapitulino@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
On 08/13/2009 04:50 PM, Luiz Capitulino wrote: > The 'i' argument type is for 32-bit only and most handlers > will use an 'int' to store its value. > > It's better to fail gracefully when the user enters a value > greater than 32-bit than to get subtle casting bugs. > > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com> > --- > monitor.c | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/monitor.c b/monitor.c > index e736de4..2052c00 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -2820,6 +2820,12 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, > } > if (get_expr(mon,&val,&p)) > goto fail; > + /* Check if 'i' is greater than 32-bit */ > + if ((c == 'i')&& ((val>> 32)& 0xffffffff)) { > + monitor_printf(mon, "\'%s\' has failed: ", cmdname); > + monitor_printf(mon, "integer is for 32-bit values\n"); > + goto fail; > + } > qdict_add_qint(qdict, key, qint_from_int64(val)); > } > What about the commands that do allow 64-bit values, like memory inspection?
On Thu, 13 Aug 2009 16:59:46 +0300 Avi Kivity <avi@redhat.com> wrote: > On 08/13/2009 04:50 PM, Luiz Capitulino wrote: > > The 'i' argument type is for 32-bit only and most handlers > > will use an 'int' to store its value. > > > > It's better to fail gracefully when the user enters a value > > greater than 32-bit than to get subtle casting bugs. > > > > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com> > > --- > > monitor.c | 6 ++++++ > > 1 files changed, 6 insertions(+), 0 deletions(-) > > > > diff --git a/monitor.c b/monitor.c > > index e736de4..2052c00 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -2820,6 +2820,12 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, > > } > > if (get_expr(mon,&val,&p)) > > goto fail; > > + /* Check if 'i' is greater than 32-bit */ > > + if ((c == 'i')&& ((val>> 32)& 0xffffffff)) { > > + monitor_printf(mon, "\'%s\' has failed: ", cmdname); > > + monitor_printf(mon, "integer is for 32-bit values\n"); > > + goto fail; > > + } > > qdict_add_qint(qdict, key, qint_from_int64(val)); > > } > > > > What about the commands that do allow 64-bit values, like memory inspection? They work with the long type ('l'), so the check will be false as it checks for 'i'.
On 08/13/2009 05:18 PM, Luiz Capitulino wrote: > On Thu, 13 Aug 2009 16:59:46 +0300 > Avi Kivity<avi@redhat.com> wrote: > > >> On 08/13/2009 04:50 PM, Luiz Capitulino wrote: >> >>> The 'i' argument type is for 32-bit only and most handlers >>> will use an 'int' to store its value. >>> >>> It's better to fail gracefully when the user enters a value >>> greater than 32-bit than to get subtle casting bugs. >>> >>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com> >>> --- >>> monitor.c | 6 ++++++ >>> 1 files changed, 6 insertions(+), 0 deletions(-) >>> >>> diff --git a/monitor.c b/monitor.c >>> index e736de4..2052c00 100644 >>> --- a/monitor.c >>> +++ b/monitor.c >>> @@ -2820,6 +2820,12 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, >>> } >>> if (get_expr(mon,&val,&p)) >>> goto fail; >>> + /* Check if 'i' is greater than 32-bit */ >>> + if ((c == 'i')&& ((val>> 32)& 0xffffffff)) { >>> + monitor_printf(mon, "\'%s\' has failed: ", cmdname); >>> + monitor_printf(mon, "integer is for 32-bit values\n"); >>> + goto fail; >>> + } >>> qdict_add_qint(qdict, key, qint_from_int64(val)); >>> } >>> >>> >> What about the commands that do allow 64-bit values, like memory inspection? >> > > They work with the long type ('l'), so the check will be false > as it checks for 'i'. > Ah, ok.
diff --git a/monitor.c b/monitor.c index e736de4..2052c00 100644 --- a/monitor.c +++ b/monitor.c @@ -2820,6 +2820,12 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, } if (get_expr(mon, &val, &p)) goto fail; + /* Check if 'i' is greater than 32-bit */ + if ((c == 'i') && ((val >> 32) & 0xffffffff)) { + monitor_printf(mon, "\'%s\' has failed: ", cmdname); + monitor_printf(mon, "integer is for 32-bit values\n"); + goto fail; + } qdict_add_qint(qdict, key, qint_from_int64(val)); } break;
The 'i' argument type is for 32-bit only and most handlers will use an 'int' to store its value. It's better to fail gracefully when the user enters a value greater than 32-bit than to get subtle casting bugs. Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- monitor.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)