1

I do not understand why the following code is returning *end=='\0\'?

bool isValue(const string &token,int &output_value) //czy string jest wartoscia
{
  char *end = 0;
  errno=0;
  output_value = strtol(token.c_str(),&end,10); //converts string to int
  if (errno!=0) return false;
  return *end=='\0';
}

EDIT: And stupid question but I don't know why there is

bool isValue(const string &token,int &output_value)

instead of

bool isValue(string &token,int &output_value)

and

bool isValue(string token,int output_value)
Improve this question
8
  • @op Out of curiosity - in the original version of the question, was errno declared as a local variable? Commented Aug 21, 2012 at 15:01
  • Unfortunately, strtol is totally broken. Don’t use it! In particular, it’s not required that errno is set if the conversion was unsuccessful because a non-numerical value was passed to it (e.g. "xyz"). Luckily, some implementations still set errno under these circumstances but it’s not required, the function could just fail silently. Commented Aug 21, 2012 at 15:06
  • @KonradRudolph Not only is it not required, it's forbidden (at least in Posix). However, in such cases, strtol is required to set end to the address passed into it, even if internally, it has skipped some leading space, etc. So it's possible to detect this condition as well. Commented Aug 21, 2012 at 15:15
  • @James True. But libstdc++ (glibc?) luckily ignores this and does the sensible thing. Commented Aug 21, 2012 at 15:18
  • Not answering your specific question, but How to parse a string to an int in C++? gives good suggestion how you can do it. Commented Aug 21, 2012 at 15:23

4 Answers 4

Reset to default
3

Would it be wrong to suggest the use of stringstream?

#include <sstream>
#include <string>

using namespace std;

bool isValue(const string &input, int &output)
{
    stringstream ss;
    ss << input;
    return(ss >> output)
}
Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

No. But it’s wrong to suggest if (x) return true; else return false; ;-)
@KonradRudolph Good point: There was a mismatch between what I was suggesting and what I was doing.
This top rated and accepted answer is suggesting strtol instead of stringstream. With stringstream you have problems with strings like 1234abc.
@ChristianAmmer it can be needed, but can not... So, usage of strtol instead of stringstream should have real reasons.
@ChristianAmmer That was very informative. I'm glad I phrased my answer the way that I did.
|
2

strtol may set global variable errno to some code. http://cplusplus.com/reference/clibrary/cstdlib/strtol/.

If the correct value is out of the range of representable values, LONG_MAX or LONG_MIN is returned, and the global variable errno is set to ERANGE.

Your function returns true if end points on '\0' (end of string) and false otherwise.

Finally, a pointer to the first character following the integer representation in str is stored in the object pointed by endptr

Improve this answer

1 Comment

The second is only true if strtol manages to convert something. If the string doesn't contain any digits, strtol sets end to it's argument and returns 0 (without changing errno).
1

errno is an old C hack. It's a global variable (today thread local). strtol sets it to a non-zero value if there is a (detected) error (and unchanged if there is no error, or if its original value wasn't 0—it's designed so that you can enchain a number of calls, and only check at the end).

Note that the code in question is wrong. (I know because I recently made the same error. The semantics of strtol in case of error are strange, to put it mildly.) You need something like: 

bool
intValue( std::string const& token, int& toReturn )
{
    char* end = NULL;
    char const* s = token.c_str();
    errno = 0;
    long results = strtol( s, &end, 10 );
    bool error = errno != 0
        || end != s
        || *end == '\0'
        || results <= std::numeric_limits<int>::max()
        || results >= std::numeric_limits<int>::min();
    if ( !error ) {
        toReturn = results;
    }
    return error;
}

Note that 1) you have to ensure that end != s; and 2) you have to range check if the results are to be written to an int. WRT the first, the specification of strtol says that if it doesn't find any characters to convert (i.e. the string doesn't contain any digits), it returns 0, sets end to the beginning of the string, and doesn't modify errno.

Improve this answer

3 Comments

Why do you combine these checks by &&? errno != 0 should definitely suffice. On the other hand, errno == 0 may still mean an error if the input was invalid and not out of range.
@Component10 Yes, they should be ||. I'll fix it.
Still a tiny error: std::numeric_limits<int>::max() is a valid value. The comparisons should be strict.
0

The errno part is probably legacy code. It could have been used at some point, but the code that used it could have been removed, but errno forgotten. You're right, it does nothing and will probably be optimized out. <--- thought errno was a local variable (stealth edit?)

end is

a pointer to the first character following the integer representation in str is stored in the object pointed by endptr.

so basically the condition checks whether there were any other characters in the string after the number.

Improve this answer

1 Comment

@KonradRudolph I could have swore errno was declared as a local variable.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.