[lvm-devel] number conversion in lvm

[Mikulas Patocka]

On Sat, 13 Jul 2013, Zdenek Kabelac wrote:

> Dne 13.7.2013 01:11, Mikulas Patocka napsal(a):
> > Hi
> > 
> > I found some strange code in _get_int_arg in ./tools/lvmcmdline.c:
> > 
> > v = strtol(val, ptr, 10);
> > 
> > if (*ptr == val)
> >          return 0;
> > 
> > av->i_value = (int32_t) v;
> > av->ui_value = (uint32_t) v;
> > av->i64_value = (int64_t) v;
> > av->ui64_value = (uint64_t) v;
> > 
> > It is taking 32-bit long value and then casting it to 64 bits. Should
> > there be strtoull instead of strtol?
> 
> It used to read only 32bit values.
> For 64bit other functions are used - i.e.  _size_arg

In tools/pvck.c there is "arg_uint64_value(cmd, labelsector_ARG,", but 
labelsector_ARG is declared with "arg(labelsector_ARG, '\0', 
"labelsector", int_arg, 0)", so there is loss of values that do not fit 
into long type.

> > BTW. how is that function supposed to handle integer overflow - it doesn't
> > seem to do that.
> > 
> 
> it returns  LONG_MIN/MAX for underflow/overflow - but assuming errno could
> be checked for ERANGE and fail command for this case instead.
> 
> Zdenek

The major problem is that the same function _get_int_arg parses both 
32-bit and 64-bit arguments. It would be natural to return error on 
integer overflow but you can't because you don't know if the caller will 
use the 32-bit value or the 64-bit value.

Another question is why there is av->i_value and av->ui_value when the 
values are the same and not dependent on the sign? What if the value fits 
into av->ui_value and doesn't fit into av->i_value - the overflow is never 
detected. This argument parser just seems totally confusing.

Mikulas