Re: [LTP] [PATCH v2 1/2] lib/safe_macros: Add SAFE_STRTOF

Re: [LTP] [PATCH v2 1/2] lib/safe_macros: Add SAFE_STRTOF

[zhaogongyi via ltp]

Hi Cyril,

> 
> Hi!
> > +float safe_strtof(const char *file, const int lineno,
> > +		  void (cleanup_fn) (void), char *str,
> > +		  float min, float max)
> > +{
> > +	float rval;
> > +	char *endptr;
> > +
> > +	errno = 0;
> > +	rval = strtof(str, &endptr);
> > +
> > +	if ((errno == ERANGE) || (rval == 0)
> > +	    || (rval == HUGE_VAL) || (rval == -HUGE_VAL)) {
> 
> This does not look right, supposedly the ERANGE is only set on overflow,
> and the rval is not guaranteed to be exact on underflow. Also rval == 0 is a
> valid result.
>

According to man 3 strtof, it seems there are some cases like:

1. strtof return the converted value as normal
2. strtof return 0 when no conversion is performed, and endptr == nptr:  'rval == 0'
3. strtof return HUGE_VAL or -HUGE_VAL when overflow:      '(rval == HUGE_VAL) || (rval == -HUGE_VAL)'
4. strtof retrun 0 and set errno to ERANGE when underflow:   'errno == ERANGE'


For 2, it seems need to add checking of endptr when 'rval == 0' like:  (rval == 0 && !strcmp(endptr, nptr)
	
Other places seem to be fine.


Regards,
Gongyi

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 1/2] lib/safe_macros: Add SAFE_STRTOF

[Cyril Hrubis]

Hi!
> According to man 3 strtof, it seems there are some cases like:
> 
> 1. strtof return the converted value as normal
> 2. strtof return 0 when no conversion is performed, and endptr == nptr:  'rval == 0'

We can ignore this case since we check that the string was consumed
later on. That's the:

+       if (endptr == str || (*endptr != '\0' && *endptr != '\n')) {
+               tst_brkm_(file, lineno, TBROK, cleanup_fn,
+                       "Invalid value: '%s'", str);
+               return 0;
+       }


> 3. strtof return HUGE_VAL or -HUGE_VAL when overflow:      '(rval == HUGE_VAL) || (rval == -HUGE_VAL)'
> 4. strtof retrun 0 and set errno to ERANGE when underflow:   'errno == ERANGE'

My manual says that the value is no larger than DBL_MIN, FLT_MIN, or
LDBL_MIN in this case. Either way I would avoid checking the returned
value in the case errno was set.

> For 2, it seems need to add checking of endptr when 'rval == 0' like:  (rval == 0 && !strcmp(endptr, nptr)

Again, no need to check the actual return value, we can just check that
the string was consumed, which we already do.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 1/2] lib/safe_macros: Add SAFE_STRTOF

[zhaogongyi via ltp]

Hi Cyril,

> 
> Hi!
> > According to man 3 strtof, it seems there are some cases like:
> >
> > 1. strtof return the converted value as normal 2. strtof return 0 when
> > no conversion is performed, and endptr == nptr:  'rval == 0'
> 
> We can ignore this case since we check that the string was consumed later
> on. That's the:
> 
> +       if (endptr == str || (*endptr != '\0' && *endptr != '\n')) {
> +               tst_brkm_(file, lineno, TBROK, cleanup_fn,
> +                       "Invalid value: '%s'", str);
> +               return 0;
> +       }
> 
> 
> > 3. strtof return HUGE_VAL or -HUGE_VAL when overflow:      '(rval ==
> HUGE_VAL) || (rval == -HUGE_VAL)'
> > 4. strtof retrun 0 and set errno to ERANGE when underflow:   'errno ==
> ERANGE'
> 
> My manual says that the value is no larger than DBL_MIN, FLT_MIN, or
> LDBL_MIN in this case. Either way I would avoid checking the returned
> value in the case errno was set.

I usually look for information here: https://pubs.opengroup.org/onlinepubs/9699919799/

> 
> > For 2, it seems need to add checking of endptr when 'rval == 0' like:
> > (rval == 0 && !strcmp(endptr, nptr)
> 
> Again, no need to check the actual return value, we can just check that the
> string was consumed, which we already do.
> 

Thanks! I have resubmit the patch as your suggestion. 

Regards,
Gongyi

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 1/2] lib/safe_macros: Add SAFE_STRTOF

[zhaogongyi via ltp]

Hi Cyril,

> 
> > Hi!
> > > +float safe_strtof(const char *file, const int lineno,
> > > +		  void (cleanup_fn) (void), char *str,
> > > +		  float min, float max)
> > > +{
> > > +	float rval;
> > > +	char *endptr;
> > > +
> > > +	errno = 0;
> > > +	rval = strtof(str, &endptr);
> > > +
> > > +	if ((errno == ERANGE) || (rval == 0)
> > > +	    || (rval == HUGE_VAL) || (rval == -HUGE_VAL)) {
> >
> > This does not look right, supposedly the ERANGE is only set on
> > overflow, and the rval is not guaranteed to be exact on underflow.
> > Also rval == 0 is a valid result.
> >
> 
> According to man 3 strtof, it seems there are some cases like:
> 
> 1. strtof return the converted value as normal 2. strtof return 0 when no
> conversion is performed, and endptr == nptr:  'rval == 0'
> 3. strtof return HUGE_VAL or -HUGE_VAL when overflow:      '(rval ==
> HUGE_VAL) || (rval == -HUGE_VAL)'
> 4. strtof retrun 0 and set errno to ERANGE when underflow:   'errno ==
> ERANGE'
> 
> 
> For 2, it seems need to add checking of endptr when 'rval == 0' like:  (rval
> == 0 && !strcmp(endptr, nptr)
> 
> Other places seem to be fine.
> 
> 

The exception cases may be described as:

 1. strtof return 0 when no conversion is performed, and endptr == nptr:  'rval == 0 && !strcmp(endptr, nptr)'

 2. strtof return HUGE_VAL or -HUGE_VAL when overflow:      '((rval ==HUGE_VAL) || (rval == -HUGE_VAL)) && errno == ERANGE'

 3. strtof retrun 0 and set errno to ERANGE when underflow:   'errno ==ERANGE && rval == 0'


So it may be write as:
	If (( errno == ERANGE && (rval == HUGE_VAL || rval == -HUGE_VAL || rval == 0))
	   || (rval == 0 && !strcmp(endptr, nptr)))

If the description as above is ok, I will resubmit the patch.

Regards,
Gongyi

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 0/2] Add handling of abnormal input for parse_opts()

[Zhao Gongyi via ltp]

Changes in v2:
1. Add range checking for SAFE_STRTOF as suggested by Cyril

Changes in v1:
1. Add SAFE_STRTOF
2. Replace atoi/atof with SAFE_STRTOL/SAFE_STRTOF to deal with abnormal
   input for parse_opts()

Zhao Gongyi (2):
  lib/safe_macros: Add SAFE_STRTOF
  lib: Replace atoi/atof with SAFE_STRTOL/SAFE_STRTOF

 include/safe_macros_fn.h  |  3 +++
 include/tst_safe_macros.h |  3 +++
 lib/safe_macros.c         | 34 ++++++++++++++++++++++++++++++++++
 lib/tst_test.c            |  9 ++++-----
 4 files changed, 44 insertions(+), 5 deletions(-)

--
2.17.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 1/2] lib/safe_macros: Add SAFE_STRTOF

[Zhao Gongyi via ltp]

Add a new macro SAFE_STRTOF, which is a safe mode of strtof().

Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
---
 include/safe_macros_fn.h  |  3 +++
 include/tst_safe_macros.h |  3 +++
 lib/safe_macros.c         | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+)

diff --git a/include/safe_macros_fn.h b/include/safe_macros_fn.h
index 114d8fd43..d256091b7 100644
--- a/include/safe_macros_fn.h
+++ b/include/safe_macros_fn.h
@@ -133,6 +133,9 @@ unsigned long safe_strtoul(const char *file, const int lineno,
                            void (cleanup_fn)(void),
                            char *str, unsigned long min, unsigned long max);

+float safe_strtof(const char *file, const int lineno,
+                 void (cleanup_fn)(void), char *str, float min, float max);
+
 long safe_sysconf(const char *file, const int lineno,
 		  void (cleanup_fn)(void), int name);

diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index ab00dd14a..0cf3d7878 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -186,6 +186,9 @@ int safe_getgroups(const char *file, const int lineno, int size, gid_t list[]);
 #define SAFE_STRTOUL(str, min, max) \
 	safe_strtoul(__FILE__, __LINE__, NULL, (str), (min), (max))

+#define SAFE_STRTOF(str, min, max) \
+	safe_strtof(__FILE__, __LINE__, NULL, (str), (min), (max))
+
 #define SAFE_SYSCONF(name) \
 	safe_sysconf(__FILE__, __LINE__, NULL, name)

diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index d8816631f..d00336748 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -21,6 +21,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <malloc.h>
+#include <math.h>
 #include "test.h"
 #include "safe_macros.h"

@@ -629,6 +630,39 @@ unsigned long safe_strtoul(const char *file, const int lineno,
 	return rval;
 }

+float safe_strtof(const char *file, const int lineno,
+		  void (cleanup_fn) (void), char *str,
+		  float min, float max)
+{
+	float rval;
+	char *endptr;
+
+	errno = 0;
+	rval = strtof(str, &endptr);
+
+	if ((errno == ERANGE) || (rval == 0)
+	    || (rval == HUGE_VAL) || (rval == -HUGE_VAL)) {
+		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
+			"strtof(%s) failed", str);
+		return rval;
+	}
+
+	if (rval > max || rval < min) {
+		tst_brkm_(file, lineno, TBROK, cleanup_fn,
+			"strtof(%s): %f is out of range %f - %f",
+			str, rval, min, max);
+		return 0;
+	}
+
+	if (endptr == str || (*endptr != '\0' && *endptr != '\n')) {
+		tst_brkm_(file, lineno, TBROK, cleanup_fn,
+			"Invalid value: '%s'", str);
+		return 0;
+	}
+
+	return rval;
+}
+
 long safe_sysconf(const char *file, const int lineno,
 		  void (cleanup_fn) (void), int name)
 {
--
2.17.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 1/2] lib/safe_macros: Add SAFE_STRTOF

[Cyril Hrubis]

Hi!
> +float safe_strtof(const char *file, const int lineno,
> +		  void (cleanup_fn) (void), char *str,
> +		  float min, float max)
> +{
> +	float rval;
> +	char *endptr;
> +
> +	errno = 0;
> +	rval = strtof(str, &endptr);
> +
> +	if ((errno == ERANGE) || (rval == 0)
> +	    || (rval == HUGE_VAL) || (rval == -HUGE_VAL)) {

This does not look right, supposedly the ERANGE is only set on overflow,
and the rval is not guaranteed to be exact on underflow. Also rval == 0
is a valid result.

We do zero errno above so I would just do:

	if (errno) {
		tst_brkm_(...);
		return rval;
	}

> +		tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn,
> +			"strtof(%s) failed", str);
> +		return rval;
> +	}
> +
> +	if (rval > max || rval < min) {
> +		tst_brkm_(file, lineno, TBROK, cleanup_fn,
> +			"strtof(%s): %f is out of range %f - %f",
> +			str, rval, min, max);
> +		return 0;
> +	}
> +
> +	if (endptr == str || (*endptr != '\0' && *endptr != '\n')) {
> +		tst_brkm_(file, lineno, TBROK, cleanup_fn,
> +			"Invalid value: '%s'", str);
> +		return 0;
> +	}
> +
> +	return rval;
> +}

Otherwise it looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp