0

I'm trying to read a string from argv. It should be something like a mathematical function like "-x^2-5" etc.

I've got something like this:

void test(char *function){
  int temp = 0;
  for (int i = 0; i < strlen(function); i++){
    if((function[i] != '+') && (function[i] != ...){
      // function[i] is a digit, but still as a char
      temp = atoi(function[i]);
    }
  }
  //... 
}
int main(int argc, char **argv){
  test(argv[1]);
  //...
}

This works quite well until the last lap of the loop. If I use printf("%c", function[i]);, it sais, it's 5.

But atoi(function[i]) gives a segfault. Why?

xXxKaddiexXx
  • 23
  • 1
  • 4
  • 1
    Please test `if(argc >= 2)` before using `argv[1]`. – Weather Vane Dec 10 '20 at 10:33
  • 2
    Turn on your compiler warnings! `function[i]` is a single `char`, not a `char*`. As such, it is implicitly cast to a pointer type and `atoi` tries to read from a near-zero memory address. Perhaps you meant `&function[i]` or `function + i` – paddy Dec 10 '20 at 10:33
  • `atoi(function[i])` is actually not valid C, so whatever compiler let this code through is bad. Be aware that most of the mainstream C compilers come in "bad mode" by default. You have to add `-std=c11 -pedantic-errors` to turn them into C compilers. Also see [“Pointer from integer/integer from pointer without a cast” issues](https://stackoverflow.com/questions/52186834/pointer-from-integer-integer-from-pointer-without-a-cast-issues). – Lundin Dec 10 '20 at 10:46

3 Answers3

0

Right. Let's first take a look at the signature of atoi.

int atoi(const char *str);

Now, what you are passing to it is function[i], and since function is of type char *, function[i] is a char. And your character is most likely not a proper character pointer.

What you'd want to pass to atoi is instead a const char *, which you can get by calling atoi(&function[i]);

sham1
  • 126
  • 1
  • 4
  • @xXxKaddiexXx Although if I were trying to parse expressions like that, I'd look into doing "proper" lexical analysis of the expression, and then using something like the "shunting yard" algorithm to create an expression tree out of the user-provided expression. Makes it easier to recognize whether a symbol is an operator, a variable like x, or a literal value like 2. Also makes it easier to evaluate. – sham1 Dec 10 '20 at 12:25
0

As already said by @sham1, atoi expects its parameter to be a C string, this is a null terminated char array, while you have a single char.

But digits are special, because they are required to have consecutive codes. So if you know that a character (say c) is a digit, its value is c - '0'. So you can write:

if((function[i] != '+') && (function[i] != ...){
  // function[i] is a digit, but still as a char
  temp = function[i] - '0';    // get the int value of a digit character
}
Serge Ballesta
  • 143,923
  • 11
  • 122
  • 252
0

Besides the bug pointed out in other answers, atoi doesn't have any reliable error handling, which is why it is one of the standard library functions that should never be used.

Just forget that you ever heard about atoi and replace it with:

char* endptr;
int result = strtol(str,&endptr,10);

if(endptr == str)
{
  // some conversion error
}

strtol is also able to convert an initial part of the string if it contains valid numbers.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • Oh no, I didn't know that. That would have saved a lot of time xD. Thank you :) – xXxKaddiexXx Dec 10 '20 at 11:17
  • @xXxKaddiexXx In this specific case, you are essentially providing something that is not a null-terminated string, so `strtol` wouldn't likely have been able to find this problem either. – Lundin Dec 10 '20 at 11:22