Spine "is_numeric" incorrectly returns TRUE for some hexadecimal values

Hi,

[Ref cacti spine v0.8.8a, same as latest HEAD.]

We're polling a Brocade fibre channel switch, which returns its
64-bit data in an octet-string format (rather than the more
sensible plain 64-bit integer).

It looks like spine should have code to handle this already with
the is_hexadecimal tests at e.g. poller.c:1107, but the values are
not getting converted to decimal. The returned values look like
"00 01 66 D7 44 4E 17 34".

It turns out that the is_numeric test (poller.c:1105) is returning
true for the above value, which means that it never falls through
- of course, it never parses as a decimal number, either... :)

The code in is_numeric tests as follows:

  is it a long (strtol) - check return code and matched length
  is it a double (strtod) - ditto return code and length
  some other test for a space, if no errno set.

The final bit of the code doesn't seem to make sense -
util.c:839-847.

It says that if there was an error (this will be from strtod
only), then return false - OK.

However, if there _wasn't_ an error then it should test to see if
the string == " " (a single space) and return false in this case,
otherwise return true.

This means that if the string "123 ABC" or "123ABC" is fed it, it
is returned as a number, but the strings "ABC123" or "ABC 123" are
not. Similarly, "00 01 66 D7 44 4E 17 34" is "numeric" because the
first part is numeric.

The first pass was that util.c:840 should, rather than reading

  if (STRIMATCH(string," ")) {

should actually read:

  if (index(string,' ')) {

to test to see if the string _contains_ a space - this would make
it fail to think that the hexadecimal number above is actually
numeric. However, thinking about it more, if a string can't be
converted to a number by strtrol _or_ strtod, then surely it's not
really a number, so this should return false?

So the entire of util.c:839-847 can be replaced with the simple
statement:

  return FALSE;

This has the desired behaviour of doing the right thing for
numbers, but failing on hex strings, which are then picked up in
poller.c by is_hexadecimal... and our graphs start to appear! :-)

I did a few test cases for different strings, of which the output
is below.

# standard code using STRIMATCH (strcasecmp):

'12345' is numeric
'+12345' is numeric
'-12345' is numeric
'-12345.123' is numeric
'12345.123' is numeric
'12345e4' is numeric
'+12345e4' is numeric
'-12345e4' is numeric
'12 STR' is numeric
'STR 12' is not numeric
'STR12' is not numeric
'12STR' is numeric
'12 AB CD 44' is numeric
'AB 00 CD 44' is not numeric

# using index instead of STRIMATCH:

'12345' is numeric
'+12345' is numeric
'-12345' is numeric
'-12345.123' is numeric
'12345.123' is numeric
'12345e4' is numeric
'+12345e4' is numeric
'-12345e4' is numeric
'12 STR' is not numeric
'STR 12' is not numeric
'STR12' is not numeric
'12STR' is numeric
'12 AB CD 44' is not numeric
'AB 00 CD 44' is not numeric

# truncated code - just return FALSE:

'12345' is numeric
'+12345' is numeric
'-12345' is numeric
'-12345.123' is numeric
'12345.123' is numeric
'12345e4' is numeric
'+12345e4' is numeric
'-12345e4' is numeric
'12 STR' is not numeric
'STR 12' is not numeric
'STR12' is not numeric
'12STR' is not numeric
'12 AB CD 44' is not numeric
'AB 00 CD 44' is not numeric


Note that when the hex number does not get converted, we also see the following
in the logs:

07/25/2014 11:54:02 AM - POLLER: Poller[0] WARNING: Poller Output Table not Empty.  Issues Found: 4, Data Sources: ...

Having fixed the conversion, this error has disappeared.

Of course, the entire is_numeric / is_hexadecimal case is a heuristic anyway,
as "00112233" could be either, but this should stop it failing in the obvious
case. Patch below.

Hope this helps!

Cheers,

Matthew



diff --git a/util.c b/util.c
index 8412026..603e1e3 100644
--- a/util.c
+++ b/util.c
@@ -836,15 +836,7 @@ int is_numeric(const char *string) {
 		end_ptr_double = NULL;
 	}
 
-	if (!errno) {
-		if (STRIMATCH(string," ")) {
-			return FALSE;
-		}else{
-			return TRUE;
-		}
-	}else{
-		return FALSE;
-	}
+	return FALSE;
 }
 
 /*! \fn int is_hexadecimal(const char *str, const short ignore_space)




-- 
Matthew Newton, Ph.D. <[email protected]>

Systems Specialist, Infrastructure Services,
I.T. Services, University of Leicester, Leicester LE1 7RH, United Kingdom

For IT help contact helpdesk extn. 2253, <[email protected]>

------------------------------------------------------------------------------
Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world's largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds