libsensors4: Use strtoul

Use strtoul() instead of parsing integers on our own.

---
 lib/data.c |   61 +++++++++----------------------------------------------------
 1 file changed, 9 insertions(+), 52 deletions(-)

Index: lm-sensors-3.0.0/lib/data.c
=================================> --- lm-sensors-3.0.0.orig/lib/data.c 2007-07-31 16:39:22.000000000 +0200
+++ lm-sensors-3.0.0/lib/data.c 2007-07-31 17:10:18.000000000 +0200

(quoted diff) +8 -46

@@ -76,7 +76,7 @@ int sensors_parse_chip_name(const char *
 {
   char *part2, *part3, *part4;
   char *name = strdup(orig_name);
-  int i;
+  char *endptr;
 
   if (! name)
     sensors_fatal_error("sensors_parse_chip_name","Allocating new name");

@@ -103,28 +103,9 @@ int sensors_parse_chip_name(const char *
   if (!strcmp(part4,"*"))
     res->addr = SENSORS_CHIP_NAME_ADDR_ANY;
   else {
-    if ((strlen(part4) > 4) || (strlen(part4) = 0))
+    res->addr = strtoul(part4, &endptr, 16);
+    if (*part4 = '\0' || *endptr != '\0' || res->addr < 0)
       goto ERROR;
-    res->addr = 0;
-    for (i = 0; ; i++) { 
-      switch (part4[i]) {
-      case '0': case '1': case '2': case '3': case '4':
-      case '5': case '6': case '7': case '8': case '9':
-        res->addr = res->addr * 16 + part4[i] - '0';
-        break;
-      case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
-        res->addr = res->addr * 16 + part4[i] - 'a' + 10;
-        break;
-      case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
-        res->addr = res->addr * 16 + part4[i] - 'A' + 10;
-        break;
-      case 0:
-        goto DONE;
-      default:
-        goto ERROR;
-      }
-    }
-DONE:;
   }
 
   /* OK. So let's look at part3. It must either be the number of the

@@ -147,22 +128,9 @@ DONE:;
   } else if (part2 && !strcmp(part2,"i2c") && !strcmp(part3,"*"))
     res->bus = SENSORS_CHIP_NAME_BUS_ANY_I2C;
   else if (part2 && !strcmp(part2,"i2c")) {
-    if ((strlen(part3) > 3) || (strlen(part3) = 0))
+    res->bus = strtoul(part3, &endptr, 10);
+    if (*part3 = '\0' || *endptr != '\0' || res->bus < 0)
       goto ERROR;
-    res->bus = 0;
-    for (i = 0; ; i++) { 
-      switch (part3[i]) {
-      case '0': case '1': case '2': case '3': case '4':
-      case '5': case '6': case '7': case '8': case '9':
-        res->bus = res->bus * 10 + part3[i] - '0';
-        break;
-      case 0:
-        goto DONE2;
-      default:
-        goto ERROR;
-      }
-    }
-DONE2:;
   } else if (res->addr = SENSORS_CHIP_NAME_ADDR_ANY) {
     res->bus = SENSORS_CHIP_NAME_BUS_ANY;
     if (part2)

@@ -188,27 +156,16 @@ ERROR:
 
 int sensors_parse_i2cbus_name(const char *name, int *res)
 {
-  int i;
+  char *endptr;
 
   if (strncmp(name,"i2c-",4)) {
     return -SENSORS_ERR_BUS_NAME;
   }
   name += 4;
-  if ((strlen(name) > 3) || (strlen(name) = 0))
+  *res = strtoul(name, &endptr, 10);
+  if (*name = '\0' || *endptr != '\0' || *res < 0)
     return -SENSORS_ERR_BUS_NAME;
-  *res = 0;
-  for (i = 0; ; i++) { 
-    switch (name[i]) {
-    case '0': case '1': case '2': case '3': case '4':
-    case '5': case '6': case '7': case '8': case '9':
-      *res = *res * 10 + name[i] - '0';
-      break;

Quoted 1 line · Jean Delvare

Use strtoul() instead of parsing integers on our own.

Erm, why all the < 0 checks on the return value of strtoul, are the variable in which the return value gets stored signed and can we have a wrap?

In that case wouldn't it be better to first store in an unsigned long and then explicitly check the limits we want to enforce?

Hi Hans,

Quoted 4 lines · Hans de Goede

Quoted 1 line · Jean Delvare

Use strtoul() instead of parsing integers on our own.

Erm, why all the < 0 checks on the return value of strtoul, are the variable in which the return value gets stored signed and can we have a wrap?

Yes, this is the reason why I added these "< 0" checks.

Quoted 2 lines

In that case wouldn't it be better to first store in an unsigned long and then explicitly check the limits we want to enforce?

There's no specific limit I want to enforce, I only want to make sure that we don't end up with a negative value due to a possible wrap. This is pure paranoia, BTW, no sane user would ever trigger it. I agree that letting the overflow happen and checking for it afterwards isn't very elegant, but it works and it is efficient, so I am a bit reluctant to make the code more complex than it needs be.

I could probably use strtol instead of strtoul if you prefer, though.