number conversion in lvm

[Zdenek Kabelac <zkabelac@redhat.com>]

Dne 15.7.2013 22:10, Mikulas Patocka napsal(a):
>
>
> 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.
>

Yep - could be some inconsistency which may need implementation of
function like   sector_arg which will accept full 64bit value.
(See args.h for labelsector_ARG)
int_arg should be left to accept only 32bit values.

Seems like noone so far tried to specify sectors reaching past 1TB when 
seeking for lvm header ;)

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

There are separate function for checking sign and accepting signed ints 
(int_arg_with_sign) - otherwise if SIGN_MINUS is detect - log error
is reported.

Zdenek