* [lm-sensors] [PATCH 4/6] libsensors4: Use strtoul
@ 2007-08-15 15:42 Jean Delvare
2007-08-15 16:59 ` Hans de Goede
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Jean Delvare @ 2007-08-15 15:42 UTC (permalink / raw)
To: lm-sensors
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
@@ -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;
- case 0:
- return 0;
- default:
- return -SENSORS_ERR_BUS_NAME;
- }
- }
+ return 0;
}
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [lm-sensors] [PATCH 4/6] libsensors4: Use strtoul
2007-08-15 15:42 [lm-sensors] [PATCH 4/6] libsensors4: Use strtoul Jean Delvare
@ 2007-08-15 16:59 ` Hans de Goede
2007-08-16 8:02 ` Jean Delvare
2007-08-16 8:42 ` Hans de Goede
2 siblings, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2007-08-15 16:59 UTC (permalink / raw)
To: lm-sensors
Jean Delvare wrote:
> 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?
Regards,
Hans
> ---
> 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
> @@ -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;
> - case 0:
> - return 0;
> - default:
> - return -SENSORS_ERR_BUS_NAME;
> - }
> - }
> + return 0;
> }
>
>
>
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [lm-sensors] [PATCH 4/6] libsensors4: Use strtoul
2007-08-15 15:42 [lm-sensors] [PATCH 4/6] libsensors4: Use strtoul Jean Delvare
2007-08-15 16:59 ` Hans de Goede
@ 2007-08-16 8:02 ` Jean Delvare
2007-08-16 8:42 ` Hans de Goede
2 siblings, 0 replies; 4+ messages in thread
From: Jean Delvare @ 2007-08-16 8:02 UTC (permalink / raw)
To: lm-sensors
Hi Hans,
On Wed, 15 Aug 2007 18:59:25 +0200, Hans de Goede wrote:
> Jean Delvare wrote:
> > 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.
> 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.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [lm-sensors] [PATCH 4/6] libsensors4: Use strtoul
2007-08-15 15:42 [lm-sensors] [PATCH 4/6] libsensors4: Use strtoul Jean Delvare
2007-08-15 16:59 ` Hans de Goede
2007-08-16 8:02 ` Jean Delvare
@ 2007-08-16 8:42 ` Hans de Goede
2 siblings, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2007-08-16 8:42 UTC (permalink / raw)
To: lm-sensors
Jean Delvare wrote:
> Hi Hans,
>
> On Wed, 15 Aug 2007 18:59:25 +0200, Hans de Goede wrote:
>> Jean Delvare wrote:
>>> 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.
>
>> 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.
>
No its fine as is then.
Regards,
Hans
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2007-08-16 8:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-08-15 15:42 [lm-sensors] [PATCH 4/6] libsensors4: Use strtoul Jean Delvare
2007-08-15 16:59 ` Hans de Goede
2007-08-16 8:02 ` Jean Delvare
2007-08-16 8:42 ` Hans de Goede
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.