convert string to double

[Alessio]

Hi,

I've written a function that convert a string into double using strtod.
Is right or there are some advice that I miss ?
Thank you.

double stringtofloat(const char *const szString, bool *const fError)
{
double dValue = 0.0;
char *p = NULL;

*fError = false;

if ( NULL == szString )
{
*fError = true;

return 0.0;
}

dValue = strtod(szString, &p);
if ( p && *p )
{
*fError = true;

return dValue;
}

return dValue;
}

 

[Ike Naar]

Hi,

I've written a function that convert a string into double using strtod.
Is right or there are some advice that I miss ?
Thank you.

double stringtofloat(const char *const szString, bool *const fError)
{
double dValue = 0.0;
char *p = NULL;

*fError = false;

if ( NULL == szString )
{
*fError = true;

return 0.0;
}

dValue = strtod(szString, &p);
if ( p && *p )
{
*fError = true;

return dValue;
}

return dValue;
}

After

bool error;
double d = stringtofloat("", &error);

d will be zero and error will be false; is that what you want?

 

[Alessio]

Ike Naar ha scritto:

After

bool error;
double d = stringtofloat("", &error);

d will be zero and error will be false; is that what you want?

Obviusly not.
I've replaced

if ( NULL == szString )

with

if ( NULL == szString || '\0' == szString[0] )

Thank you for this bug.

 

[Ben Bacarisse]

I've written a function that convert a string into double using strtod.
Is right or there are some advice that I miss ?
Thank you.

double stringtofloat(const char *const szString, bool *const fError)
{
double dValue = 0.0;
char *p = NULL;

*fError = false;

if ( NULL == szString )
{
*fError = true;

return 0.0;
}

dValue = strtod(szString, &p);
if ( p && *p )

There is really no point in the first test here. p will be non-null
in all cases because you check szString first. There are three error
cases reported by the strtoX functions:

p == szString means no conversion was done
*p != '\0' means there were "left over" characters
errno != 0 means something else want wrong

strtod only ever sets errno to ERANGE but that hardly matters to
you -- any non-zero errno means something went wrong. You are testing
for only the second of these three.

 

[Alessio]

Ben Bacarisse ha scritto:

There is really no point in the first test here. p will be non-null
in all cases because you check szString first. There are three error
cases reported by the strtoX functions:

p == szString means no conversion was done
*p != '\0' means there were "left over" characters
errno != 0 means something else want wrong

strtod only ever sets errno to ERANGE but that hardly matters to
you -- any non-zero errno means something went wrong. You are testing
for only the second of these three.

Uhm...tons of errors in just ten lines of code.
I've rewritten it, everything should be fine now.

static double stringtofloat(const char *const szString, bool *const fError)
{
double dValue = 0.0;
char *p = NULL;

*fError = false;

if ( NULL == szString )
{
*fError = true;

return 0.0;
}

errno = 0;
dValue = strtod(szString, &p);
if ( szString == p || *p || 0 != errno )
{
*fError = true;
}

return dValue;
}

thank you,
Alessio

 

[Ben Bacarisse]

Uhm...tons of errors in just ten lines of code.

Hardly. Still, being self-critical is not a bad attribute for a
programmer.

I've rewritten it, everything should be fine now.

static double stringtofloat(const char *const szString, bool *const fError)
{
double dValue = 0.0;
char *p = NULL;

*fError = false;

if ( NULL == szString )
{
*fError = true;

return 0.0;
}

errno = 0;
dValue = strtod(szString, &p);
if ( szString == p || *p || 0 != errno )

All fine now, but on the question of style, there is no "safety
advantage" in writing szString == p rather than p == szString (I know
why you write 0 != errno) so I'd write the latter. Also, if you are
happy to write just '*p', why not just 'errno'? I.e. it would be more
consistent to write:

if (szString == p || *p || errno)
or
if (szString == p || 0 != *p || 0 != errno)

As I say, a small matter of style so don't even feel you have to
reply! What you wrote is perfectly understandable.

 

[Ben Bacarisse]

Ike Naar ha scritto:

After

bool error;
double d = stringtofloat("", &error);

d will be zero and error will be false; is that what you want?

Obviusly not.
I've replaced

if ( NULL == szString )

with

if ( NULL == szString || '\0' == szString[0] )

This post pre-dates the one in reply to my post so you have probably
already noticed that you don't need to do this.

There are lots of conditions that cause strtod to do no conversion at
all but they are all signaled by setting p to szString. The case of
an empty string was the only one your previous code missed but if you
are going to test for p == szString you may as well let that test
catch the empty string as well.

 

[Mark]

There is really no point in the first test here. p will be non-null
in all cases because you check szString first. There are three error
cases reported by the strtoX functions:

p == szString means no conversion was done
*p != '\0' means there were "left over" characters
errno != 0 means something else want wrong

double strtod(const char *nptr, char **endptr);

I don't actually understand the point of having the second parameter
'endptr' in the function, where is may be useful and how can be exploited?

 

[Ben Bacarisse]

double strtod(const char *nptr, char **endptr);

I don't actually understand the point of having the second parameter
endptr' in the function, where is may be useful and how can be
exploited?

When everything works, the second parameter can be used to find out
what follows a number. For example, if you have number with a unit it
is obviously helpful to be told where the number finished. Given

char *input = "24cm", *end;

after a call like

x = strtod(input, &end);

x will be 24 and end will point to the 'c' of "cm". Try to write the
code to find the unit without strtod and you will see how helpful it
is.

Another example is simply scanning a string with consecutive numbers.
If you get told where one number ends you know what pointer to pass to
strtod to get the next one.

The pointer is also used when things go wrong. In the call above, if
end == input after the call we know that no number was found in the
string.

 

[Barry Schwarz]

snip

I've rewritten it, everything should be fine now.

static double stringtofloat(const char *const szString, bool *const fError)
{
double dValue = 0.0;
char *p = NULL;

*fError = false;

if ( NULL == szString )
{
*fError = true;

return 0.0;
}

errno = 0;
dValue = strtod(szString, &p);
if ( szString == p || *p || 0 != errno )
{
*fError = true;
}

return dValue;
}

*fError is set true if
1 - szString is NULL
2 - szString points to an empty string
3 - strtod finds an invalid character
4 - errno is set

You don't pass the value of p back to the calling function.

This makes it unnecessarily difficult for the calling function to
distinguish between case 1, case 2, case 3 with an invalid character
at the start ("abc"), case 3 with an invalid character after 0
("0.0a"), or case 4 with a valid string ("0.0") but errno set by a
previous function call.

 

[Mark]

When everything works, the second parameter can be used to find out
what follows a number. For example, if you have number with a unit it
is obviously helpful to be told where the number finished. Given

char *input = "24cm", *end;

after a call like

x = strtod(input, &end);

x will be 24 and end will point to the 'c' of "cm". Try to write the
code to find the unit without strtod and you will see how helpful it
is.

Indeed, it makes perfect sense. Thanks a lot for clarifications.