[v2,1/2] util: xatol() and xatoi() helpers introduced

Submitted by Stanislav Kinsburskiy on Sept. 13, 2017, 1:48 p.m.

Details

Message ID 20170913134850.51546.13723.stgit@skinsbursky-vz7.qa.sw.ru
State New
Series "autofs: fix "pipe_ino" option parsing"
Headers show

Commit Message

Stanislav Kinsburskiy Sept. 13, 2017, 1:48 p.m.
These helpers are safe versions of atol() and atoi() respectively.
And they check for overflow and NAN errors

Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
---
 criu/include/util.h |    3 +++
 criu/util.c         |   38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

Patch hide | download patch | download mbox

diff --git a/criu/include/util.h b/criu/include/util.h
index 1fc20db..20684cc 100644
--- a/criu/include/util.h
+++ b/criu/include/util.h
@@ -280,6 +280,9 @@  void tcp_cork(int sk, bool on);
 
 const char *ns_to_string(unsigned int ns);
 
+int xatol(const char *string, long *number);
+int xatoi(const char *string, int *number);
+
 char *xstrcat(char *str, const char *fmt, ...)
 	__attribute__ ((__format__ (__printf__, 2, 3)));
 char *xsprintf(const char *fmt, ...)
diff --git a/criu/util.c b/criu/util.c
index b5a161f..7d60617 100644
--- a/criu/util.c
+++ b/criu/util.c
@@ -58,6 +58,44 @@ 
 
 #define VMA_OPT_LEN	128
 
+static int xatol_base(const char *string, long *number, int base)
+{
+	char *endptr;
+	long nr;
+
+	errno = 0;
+	nr = strtol(string, &endptr, base);
+	if ((errno == ERANGE && (nr == LONG_MAX || nr == LONG_MIN))
+			|| (errno != 0 && nr == 0)) {
+		pr_perror("failed to convert string");
+		return -EINVAL;
+	}
+
+	if ((endptr == string) || (*endptr != '\0')) {
+		pr_err("String is not a number: '%s'\n", string);
+		return -EINVAL;
+	}
+	*number = nr;
+	return 0;
+}
+
+int xatol(const char *string, long *number)
+{
+	return xatol_base(string, number, 10);
+}
+
+
+int xatoi(const char *string, int *number)
+{
+	long tmp;
+	int err;
+
+	err = xatol(string, &tmp);
+	if (!err)
+		*number = (int)tmp;
+	return err;
+}
+
 /*
  * This function reallocates passed str pointer.
  * It means:

Comments

Andrei Vagin Sept. 22, 2017, 10:30 p.m.
On Wed, Sep 13, 2017 at 04:48:50PM +0300, Stanislav Kinsburskiy wrote:
> These helpers are safe versions of atol() and atoi() respectively.
> And they check for overflow and NAN errors
> 
> Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
> ---
>  criu/include/util.h |    3 +++
>  criu/util.c         |   38 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/criu/include/util.h b/criu/include/util.h
> index 1fc20db..20684cc 100644
> --- a/criu/include/util.h
> +++ b/criu/include/util.h
> @@ -280,6 +280,9 @@ void tcp_cork(int sk, bool on);
>  
>  const char *ns_to_string(unsigned int ns);
>  
> +int xatol(const char *string, long *number);
> +int xatoi(const char *string, int *number);
> +
>  char *xstrcat(char *str, const char *fmt, ...)
>  	__attribute__ ((__format__ (__printf__, 2, 3)));
>  char *xsprintf(const char *fmt, ...)
> diff --git a/criu/util.c b/criu/util.c
> index b5a161f..7d60617 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -58,6 +58,44 @@
>  
>  #define VMA_OPT_LEN	128
>  
> +static int xatol_base(const char *string, long *number, int base)
> +{
> +	char *endptr;
> +	long nr;
> +
> +	errno = 0;
> +	nr = strtol(string, &endptr, base);
> +	if ((errno == ERANGE && (nr == LONG_MAX || nr == LONG_MIN))
> +			|| (errno != 0 && nr == 0)) {
> +		pr_perror("failed to convert string");

It is better to print this string in a error message

> +		return -EINVAL;
> +	}
> +
> +	if ((endptr == string) || (*endptr != '\0')) {
> +		pr_err("String is not a number: '%s'\n", string);
> +		return -EINVAL;
> +	}
> +	*number = nr;
> +	return 0;
> +}
> +
> +int xatol(const char *string, long *number)
> +{
> +	return xatol_base(string, number, 10);
> +}
> +
> +
> +int xatoi(const char *string, int *number)
> +{
> +	long tmp;
> +	int err;
> +
> +	err = xatol(string, &tmp);

Should we check that tmp is in [INT_MIN, INT_MAX]?
> +	if (!err)
> +		*number = (int)tmp;
> +	return err;
> +}
> +
>  /*
>   * This function reallocates passed str pointer.
>   * It means:
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
Stanislav Kinsburskiy Sept. 25, 2017, 9:07 a.m.
23.09.2017 00:30, Andrei Vagin пишет:
> On Wed, Sep 13, 2017 at 04:48:50PM +0300, Stanislav Kinsburskiy wrote:
>> These helpers are safe versions of atol() and atoi() respectively.
>> And they check for overflow and NAN errors
>>
>> Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
>> ---
>>  criu/include/util.h |    3 +++
>>  criu/util.c         |   38 ++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 41 insertions(+)
>>
>> diff --git a/criu/include/util.h b/criu/include/util.h
>> index 1fc20db..20684cc 100644
>> --- a/criu/include/util.h
>> +++ b/criu/include/util.h
>> @@ -280,6 +280,9 @@ void tcp_cork(int sk, bool on);
>>  
>>  const char *ns_to_string(unsigned int ns);
>>  
>> +int xatol(const char *string, long *number);
>> +int xatoi(const char *string, int *number);
>> +
>>  char *xstrcat(char *str, const char *fmt, ...)
>>  	__attribute__ ((__format__ (__printf__, 2, 3)));
>>  char *xsprintf(const char *fmt, ...)
>> diff --git a/criu/util.c b/criu/util.c
>> index b5a161f..7d60617 100644
>> --- a/criu/util.c
>> +++ b/criu/util.c
>> @@ -58,6 +58,44 @@
>>  
>>  #define VMA_OPT_LEN	128
>>  
>> +static int xatol_base(const char *string, long *number, int base)
>> +{
>> +	char *endptr;
>> +	long nr;
>> +
>> +	errno = 0;
>> +	nr = strtol(string, &endptr, base);
>> +	if ((errno == ERANGE && (nr == LONG_MAX || nr == LONG_MIN))
>> +			|| (errno != 0 && nr == 0)) {
>> +		pr_perror("failed to convert string");
> 
> It is better to print this string in a error message
> 


Well,on the other hand how we can distinguish between this error case above and EINVAL, returned below, when string is not a number?


>> +		return -EINVAL;
>> +	}
>> +
>> +	if ((endptr == string) || (*endptr != '\0')) {
>> +		pr_err("String is not a number: '%s'\n", string);
>> +		return -EINVAL;
>> +	}
>> +	*number = nr;
>> +	return 0;
>> +}
>> +
>> +int xatol(const char *string, long *number)
>> +{
>> +	return xatol_base(string, number, 10);
>> +}
>> +
>> +
>> +int xatoi(const char *string, int *number)
>> +{
>> +	long tmp;
>> +	int err;
>> +
>> +	err = xatol(string, &tmp);
> 
> Should we check that tmp is in [INT_MIN, INT_MAX]?


Well, looks like we shouldn't. Atoi will return interger even if string contains long.


>> +	if (!err)
>> +		*number = (int)tmp;
>> +	return err;
>> +}
>> +
>>  /*
>>   * This function reallocates passed str pointer.
>>   * It means:
>>
>> _______________________________________________
>> CRIU mailing list
>> CRIU@openvz.org
>> https://lists.openvz.org/mailman/listinfo/criu
Andrei Vagin Sept. 25, 2017, 6:19 p.m.
On Mon, Sep 25, 2017 at 11:07:11AM +0200, Stanislav Kinsburskiy wrote:
> 
> 
> 23.09.2017 00:30, Andrei Vagin пишет:
> > On Wed, Sep 13, 2017 at 04:48:50PM +0300, Stanislav Kinsburskiy wrote:
> >> These helpers are safe versions of atol() and atoi() respectively.
> >> And they check for overflow and NAN errors
> >>
> >> Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
> >> ---
> >>  criu/include/util.h |    3 +++
> >>  criu/util.c         |   38 ++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 41 insertions(+)
> >>
> >> diff --git a/criu/include/util.h b/criu/include/util.h
> >> index 1fc20db..20684cc 100644
> >> --- a/criu/include/util.h
> >> +++ b/criu/include/util.h
> >> @@ -280,6 +280,9 @@ void tcp_cork(int sk, bool on);
> >>  
> >>  const char *ns_to_string(unsigned int ns);
> >>  
> >> +int xatol(const char *string, long *number);
> >> +int xatoi(const char *string, int *number);
> >> +
> >>  char *xstrcat(char *str, const char *fmt, ...)
> >>  	__attribute__ ((__format__ (__printf__, 2, 3)));
> >>  char *xsprintf(const char *fmt, ...)
> >> diff --git a/criu/util.c b/criu/util.c
> >> index b5a161f..7d60617 100644
> >> --- a/criu/util.c
> >> +++ b/criu/util.c
> >> @@ -58,6 +58,44 @@
> >>  
> >>  #define VMA_OPT_LEN	128
> >>  
> >> +static int xatol_base(const char *string, long *number, int base)
> >> +{
> >> +	char *endptr;
> >> +	long nr;
> >> +
> >> +	errno = 0;
> >> +	nr = strtol(string, &endptr, base);
> >> +	if ((errno == ERANGE && (nr == LONG_MAX || nr == LONG_MIN))
> >> +			|| (errno != 0 && nr == 0)) {
> >> +		pr_perror("failed to convert string");
> > 
> > It is better to print this string in a error message
> > 
> 
> 
> Well,on the other hand how we can distinguish between this error case above and EINVAL, returned below, when string is not a number?

I am not sure that I understand what you mean. Error messages are
different, is it not enough?

> 
> 
> >> +		return -EINVAL;
> >> +	}
> >> +
> >> +	if ((endptr == string) || (*endptr != '\0')) {
> >> +		pr_err("String is not a number: '%s'\n", string);
> >> +		return -EINVAL;
> >> +	}
> >> +	*number = nr;
> >> +	return 0;
> >> +}
> >> +
> >> +int xatol(const char *string, long *number)
> >> +{
> >> +	return xatol_base(string, number, 10);
> >> +}
> >> +
> >> +
> >> +int xatoi(const char *string, int *number)
> >> +{
> >> +	long tmp;
> >> +	int err;
> >> +
> >> +	err = xatol(string, &tmp);
> > 
> > Should we check that tmp is in [INT_MIN, INT_MAX]?
> 
> 
> Well, looks like we shouldn't. Atoi will return interger even if string contains long.

So why do we need to check that a value is in [LONG_MIN, LONG_MAX] for
xatol? Why do we need these hellpers?

> 
> 
> >> +	if (!err)
> >> +		*number = (int)tmp;
> >> +	return err;
> >> +}
> >> +
> >>  /*
> >>   * This function reallocates passed str pointer.
> >>   * It means:
> >>
> >> _______________________________________________
> >> CRIU mailing list
> >> CRIU@openvz.org
> >> https://lists.openvz.org/mailman/listinfo/criu
Stanislav Kinsburskiy Sept. 26, 2017, 9:16 a.m.
25.09.2017 20:19, Andrei Vagin пишет:
> On Mon, Sep 25, 2017 at 11:07:11AM +0200, Stanislav Kinsburskiy wrote:
>>
>>
>> 23.09.2017 00:30, Andrei Vagin пишет:
>>> On Wed, Sep 13, 2017 at 04:48:50PM +0300, Stanislav Kinsburskiy wrote:
>>>> These helpers are safe versions of atol() and atoi() respectively.
>>>> And they check for overflow and NAN errors
>>>>
>>>> Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
>>>> ---
>>>>  criu/include/util.h |    3 +++
>>>>  criu/util.c         |   38 ++++++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 41 insertions(+)
>>>>
>>>> diff --git a/criu/include/util.h b/criu/include/util.h
>>>> index 1fc20db..20684cc 100644
>>>> --- a/criu/include/util.h
>>>> +++ b/criu/include/util.h
>>>> @@ -280,6 +280,9 @@ void tcp_cork(int sk, bool on);
>>>>  
>>>>  const char *ns_to_string(unsigned int ns);
>>>>  
>>>> +int xatol(const char *string, long *number);
>>>> +int xatoi(const char *string, int *number);
>>>> +
>>>>  char *xstrcat(char *str, const char *fmt, ...)
>>>>  	__attribute__ ((__format__ (__printf__, 2, 3)));
>>>>  char *xsprintf(const char *fmt, ...)
>>>> diff --git a/criu/util.c b/criu/util.c
>>>> index b5a161f..7d60617 100644
>>>> --- a/criu/util.c
>>>> +++ b/criu/util.c
>>>> @@ -58,6 +58,44 @@
>>>>  
>>>>  #define VMA_OPT_LEN	128
>>>>  
>>>> +static int xatol_base(const char *string, long *number, int base)
>>>> +{
>>>> +	char *endptr;
>>>> +	long nr;
>>>> +
>>>> +	errno = 0;
>>>> +	nr = strtol(string, &endptr, base);
>>>> +	if ((errno == ERANGE && (nr == LONG_MAX || nr == LONG_MIN))
>>>> +			|| (errno != 0 && nr == 0)) {
>>>> +		pr_perror("failed to convert string");
>>>
>>> It is better to print this string in a error message
>>>
>>
>>
>> Well,on the other hand how we can distinguish between this error case above and EINVAL, returned below, when string is not a number?
> 
> I am not sure that I understand what you mean. Error messages are
> different, is it not enough?
> 

They are not. Function strtol can return EINVAL.
And EINVAL is also returned below on endptr check.

>>
>>
>>>> +		return -EINVAL;
>>>> +	}
>>>> +
>>>> +	if ((endptr == string) || (*endptr != '\0')) {
>>>> +		pr_err("String is not a number: '%s'\n", string);
>>>> +		return -EINVAL;

^^^^^^^^^^^^^^^^^^^^^^^^^^ here.

But here it can be converted to some other error though... ENOTSUP for instance, if you insist on moving the print outside the helper.


>>>> +	}
>>>> +	*number = nr;
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +int xatol(const char *string, long *number)
>>>> +{
>>>> +	return xatol_base(string, number, 10);
>>>> +}
>>>> +
>>>> +
>>>> +int xatoi(const char *string, int *number)
>>>> +{
>>>> +	long tmp;
>>>> +	int err;
>>>> +
>>>> +	err = xatol(string, &tmp);
>>>
>>> Should we check that tmp is in [INT_MIN, INT_MAX]?
>>
>>
>> Well, looks like we shouldn't. Atoi will return interger even if string contains long.
> 
> So why do we need to check that a value is in [LONG_MIN, LONG_MAX] for
> xatol? Why do we need these hellpers?
> 

1) In case of xatol we check the string can be converted in arch-size variable.
2) In case of xatoi it's not needed, because number can be converted to int by simply zeroing high 32 bits. You don't check number for int, when casting like this "(int)long_number", don't you?
3) These helpers do convert strings to numbers *properly* and taking errors into account.
I understand, that CRIU relies on the fact that kernel is much less error-prone and if we know, that is has to return int (long) we can rely on it. But I don't think that it's a safe approach. Do you?

>>
>>
>>>> +	if (!err)
>>>> +		*number = (int)tmp;
>>>> +	return err;
>>>> +}
>>>> +
>>>>  /*
>>>>   * This function reallocates passed str pointer.
>>>>   * It means:
>>>>
>>>> _______________________________________________
>>>> CRIU mailing list
>>>> CRIU@openvz.org
>>>> https://lists.openvz.org/mailman/listinfo/criu
Andrei Vagin Sept. 26, 2017, 8:30 p.m.
On Tue, Sep 26, 2017 at 11:16:14AM +0200, Stanislav Kinsburskiy wrote:
> 
> 
> 25.09.2017 20:19, Andrei Vagin пишет:
> > On Mon, Sep 25, 2017 at 11:07:11AM +0200, Stanislav Kinsburskiy wrote:
> >>
> >>
> >> 23.09.2017 00:30, Andrei Vagin пишет:
> >>> On Wed, Sep 13, 2017 at 04:48:50PM +0300, Stanislav Kinsburskiy wrote:
> >>>> These helpers are safe versions of atol() and atoi() respectively.
> >>>> And they check for overflow and NAN errors
> >>>>
> >>>> Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
> >>>> ---
> >>>>  criu/include/util.h |    3 +++
> >>>>  criu/util.c         |   38 ++++++++++++++++++++++++++++++++++++++
> >>>>  2 files changed, 41 insertions(+)
> >>>>
> >>>> diff --git a/criu/include/util.h b/criu/include/util.h
> >>>> index 1fc20db..20684cc 100644
> >>>> --- a/criu/include/util.h
> >>>> +++ b/criu/include/util.h
> >>>> @@ -280,6 +280,9 @@ void tcp_cork(int sk, bool on);
> >>>>  
> >>>>  const char *ns_to_string(unsigned int ns);
> >>>>  
> >>>> +int xatol(const char *string, long *number);
> >>>> +int xatoi(const char *string, int *number);
> >>>> +
> >>>>  char *xstrcat(char *str, const char *fmt, ...)
> >>>>  	__attribute__ ((__format__ (__printf__, 2, 3)));
> >>>>  char *xsprintf(const char *fmt, ...)
> >>>> diff --git a/criu/util.c b/criu/util.c
> >>>> index b5a161f..7d60617 100644
> >>>> --- a/criu/util.c
> >>>> +++ b/criu/util.c
> >>>> @@ -58,6 +58,44 @@
> >>>>  
> >>>>  #define VMA_OPT_LEN	128
> >>>>  
> >>>> +static int xatol_base(const char *string, long *number, int base)
> >>>> +{
> >>>> +	char *endptr;
> >>>> +	long nr;
> >>>> +
> >>>> +	errno = 0;
> >>>> +	nr = strtol(string, &endptr, base);
> >>>> +	if ((errno == ERANGE && (nr == LONG_MAX || nr == LONG_MIN))
> >>>> +			|| (errno != 0 && nr == 0)) {
> >>>> +		pr_perror("failed to convert string");

pr_perror("failed to convert string: %s", string);

> >>>
> >>> It is better to print this string in a error message
> >>>
> >>
> >>
> >> Well,on the other hand how we can distinguish between this error case above and EINVAL, returned below, when string is not a number?
> > 
> > I am not sure that I understand what you mean. Error messages are
> > different, is it not enough?
> > 
> 
> They are not. Function strtol can return EINVAL.
> And EINVAL is also returned below on endptr check.

I sugested to print a string in a error message, why you started talking
about error codes? Why don't you return ERANGE here?

> 
> >>
> >>
> >>>> +		return -EINVAL;
> >>>> +	}
> >>>> +
> >>>> +	if ((endptr == string) || (*endptr != '\0')) {
> >>>> +		pr_err("String is not a number: '%s'\n", string);
> >>>> +		return -EINVAL;
> 
> ^^^^^^^^^^^^^^^^^^^^^^^^^^ here.
> 
> But here it can be converted to some other error though... ENOTSUP for instance, if you insist on moving the print outside the helper.
> 
> 
> >>>> +	}
> >>>> +	*number = nr;
> >>>> +	return 0;
> >>>> +}
> >>>> +
> >>>> +int xatol(const char *string, long *number)
> >>>> +{
> >>>> +	return xatol_base(string, number, 10);
> >>>> +}
> >>>> +
> >>>> +
> >>>> +int xatoi(const char *string, int *number)
> >>>> +{
> >>>> +	long tmp;
> >>>> +	int err;
> >>>> +
> >>>> +	err = xatol(string, &tmp);
> >>>
> >>> Should we check that tmp is in [INT_MIN, INT_MAX]?
> >>
> >>
> >> Well, looks like we shouldn't. Atoi will return interger even if string contains long.
> > 
> > So why do we need to check that a value is in [LONG_MIN, LONG_MAX] for
> > xatol? Why do we need these hellpers?
> > 
> 
> 1) In case of xatol we check the string can be converted in arch-size variable.
> 2) In case of xatoi it's not needed, because number can be converted to int by simply zeroing high 32 bits. You don't check number for int, when casting like this "(int)long_number", don't you?

It is not convince me. xatoi and xtol have to be symetrical, if one of
them check a range, another one should check a range too (IMHO)

> 3) These helpers do convert strings to numbers *properly* and taking errors into account.
> I understand, that CRIU relies on the fact that kernel is much less error-prone and if we know, that is has to return int (long) we can rely on it. But I don't think that it's a safe approach. Do you?

I am agree and this point contradicts with your explanation.

If the kernel has to return int, but it returns a value bigger than
INT_MAX, xatoi has to return an error, doesn't it?

> 
> >>
> >>
> >>>> +	if (!err)
> >>>> +		*number = (int)tmp;
> >>>> +	return err;
> >>>> +}
> >>>> +
> >>>>  /*
> >>>>   * This function reallocates passed str pointer.
> >>>>   * It means:
> >>>>
> >>>> _______________________________________________
> >>>> CRIU mailing list
> >>>> CRIU@openvz.org
> >>>> https://lists.openvz.org/mailman/listinfo/criu
Stanislav Kinsburskiy Sept. 27, 2017, 9:12 a.m.
26.09.2017 22:30, Andrei Vagin пишет:
> On Tue, Sep 26, 2017 at 11:16:14AM +0200, Stanislav Kinsburskiy wrote:
>>
>>
>> 25.09.2017 20:19, Andrei Vagin пишет:
>>> On Mon, Sep 25, 2017 at 11:07:11AM +0200, Stanislav Kinsburskiy wrote:
>>>>
>>>>
>>>> 23.09.2017 00:30, Andrei Vagin пишет:
>>>>> On Wed, Sep 13, 2017 at 04:48:50PM +0300, Stanislav Kinsburskiy wrote:
>>>>>> These helpers are safe versions of atol() and atoi() respectively.
>>>>>> And they check for overflow and NAN errors
>>>>>>
>>>>>> Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
>>>>>> ---
>>>>>>  criu/include/util.h |    3 +++
>>>>>>  criu/util.c         |   38 ++++++++++++++++++++++++++++++++++++++
>>>>>>  2 files changed, 41 insertions(+)
>>>>>>
>>>>>> diff --git a/criu/include/util.h b/criu/include/util.h
>>>>>> index 1fc20db..20684cc 100644
>>>>>> --- a/criu/include/util.h
>>>>>> +++ b/criu/include/util.h
>>>>>> @@ -280,6 +280,9 @@ void tcp_cork(int sk, bool on);
>>>>>>  
>>>>>>  const char *ns_to_string(unsigned int ns);
>>>>>>  
>>>>>> +int xatol(const char *string, long *number);
>>>>>> +int xatoi(const char *string, int *number);
>>>>>> +
>>>>>>  char *xstrcat(char *str, const char *fmt, ...)
>>>>>>  	__attribute__ ((__format__ (__printf__, 2, 3)));
>>>>>>  char *xsprintf(const char *fmt, ...)
>>>>>> diff --git a/criu/util.c b/criu/util.c
>>>>>> index b5a161f..7d60617 100644
>>>>>> --- a/criu/util.c
>>>>>> +++ b/criu/util.c
>>>>>> @@ -58,6 +58,44 @@
>>>>>>  
>>>>>>  #define VMA_OPT_LEN	128
>>>>>>  
>>>>>> +static int xatol_base(const char *string, long *number, int base)
>>>>>> +{
>>>>>> +	char *endptr;
>>>>>> +	long nr;
>>>>>> +
>>>>>> +	errno = 0;
>>>>>> +	nr = strtol(string, &endptr, base);
>>>>>> +	if ((errno == ERANGE && (nr == LONG_MAX || nr == LONG_MIN))
>>>>>> +			|| (errno != 0 && nr == 0)) {
>>>>>> +		pr_perror("failed to convert string");
> 
> pr_perror("failed to convert string: %s", string);
> 

Ok, now got it.

>>>>>
>>>>> It is better to print this string in a error message
>>>>>
>>>>
>>>>
>>>> Well,on the other hand how we can distinguish between this error case above and EINVAL, returned below, when string is not a number?
>>>
>>> I am not sure that I understand what you mean. Error messages are
>>> different, is it not enough?
>>>
>>
>> They are not. Function strtol can return EINVAL.
>> And EINVAL is also returned below on endptr check.
> 
> I sugested to print a string in a error message, why you started talking
> about error codes? Why don't you return ERANGE here?
> 

It was misunderstanding. Will add the string to the error message.

>>
>>>>
>>>>
>>>>>> +		return -EINVAL;
>>>>>> +	}
>>>>>> +
>>>>>> +	if ((endptr == string) || (*endptr != '\0')) {
>>>>>> +		pr_err("String is not a number: '%s'\n", string);
>>>>>> +		return -EINVAL;
>>
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^ here.
>>
>> But here it can be converted to some other error though... ENOTSUP for instance, if you insist on moving the print outside the helper.
>>
>>
>>>>>> +	}
>>>>>> +	*number = nr;
>>>>>> +	return 0;
>>>>>> +}
>>>>>> +
>>>>>> +int xatol(const char *string, long *number)
>>>>>> +{
>>>>>> +	return xatol_base(string, number, 10);
>>>>>> +}
>>>>>> +
>>>>>> +
>>>>>> +int xatoi(const char *string, int *number)
>>>>>> +{
>>>>>> +	long tmp;
>>>>>> +	int err;
>>>>>> +
>>>>>> +	err = xatol(string, &tmp);
>>>>>
>>>>> Should we check that tmp is in [INT_MIN, INT_MAX]?
>>>>
>>>>
>>>> Well, looks like we shouldn't. Atoi will return interger even if string contains long.
>>>
>>> So why do we need to check that a value is in [LONG_MIN, LONG_MAX] for
>>> xatol? Why do we need these hellpers?
>>>
>>
>> 1) In case of xatol we check the string can be converted in arch-size variable.
>> 2) In case of xatoi it's not needed, because number can be converted to int by simply zeroing high 32 bits. You don't check number for int, when casting like this "(int)long_number", don't you?
> 
> It is not convince me. xatoi and xtol have to be symetrical, if one of
> them check a range, another one should check a range too (IMHO)
> 

Well, I'll add the check, if you wish.

>> 3) These helpers do convert strings to numbers *properly* and taking errors into account.
>> I understand, that CRIU relies on the fact that kernel is much less error-prone and if we know, that is has to return int (long) we can rely on it. But I don't think that it's a safe approach. Do you?
> 
> I am agree and this point contradicts with your explanation.
> 
> If the kernel has to return int, but it returns a value bigger than
> INT_MAX, xatoi has to return an error, doesn't it?
> 

Well, let's do it this way.