1

I am trying to write RPN calculator, but I've been struggling with getting arguments from user. What I want to do is: take whole line, split it to tokens and put it to array. I have something like below, but it works only once. On the second while-loop performing line is taken, but arr==NULL

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

#define MAX_str 100

char* getLine(){
  char* line = malloc(MAX_str*sizeof(char*));
  return fgets(line, MAX_str,stdin);
}

char** toArray(char* line){
  int i=0;
  char** array= malloc(MAX_str*sizeof(char*));
  array[i] = strtok(line," \t");
  while(array[i]!=NULL){
    array[++i] = strtok(NULL,"");
  }
  return array;
}

int main(){
  char* linia;
  char** arr;
  int i=0,end=0;

  while(!end){
    linia=getLine();
    arr = toArray(linia);
    while(arr[i]!=NULL){
      printf("%s\n",arr[i]);
      i++;
    }
  free(linia);
  free(arr);
  }
}

Secondly, strtok splits only on two tokens, for example

>1 2 3

gives:

>1
>2 3
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
pjoter
  • 163
  • 2
  • 13
  • Besides the big mess the code creates allocation to much (`sizeof(char*)`), to few (`sizeof(line)`), and missing the deallocate, you want to tell *each* call to `strdup()` to do the same, namely to tokenise by `" \t"`. – alk Jan 08 '18 at 16:09
  • moorzyn, Curious: Why does code use `sizeof(line)` in `char** array = malloc(MAX_str*sizeof(line));`? – chux - Reinstate Monica Jan 08 '18 at 16:15
  • Hint: `""` in `array[++i] = strtok(NULL,"");` is a very small token list. – chux - Reinstate Monica Jan 08 '18 at 16:17
  • @allk Re: [to few (sizeof(line))](https://stackoverflow.com/questions/48153911/how-to-get-line-from-stdin-and-split-it-to-tokens-c#comment83286792_48153911). this looks unusual, yet `malloc(MAX_str*sizeof(line));` does not appear too few. Your thoughts? – chux - Reinstate Monica Jan 08 '18 at 16:21
  • Well, I am just starting learning C, so this is why it look so bad. @chux I meant to be `char*` there, not `line`. I don't know why I put it there. I have edited the code. – pjoter Jan 08 '18 at 16:40
  • @moorzyn Tip: instead of `char** array= malloc(MAX_str*sizeof(char*));` and hope the correct type was used, code `char** array= malloc(MAX_str*sizeof *array);` and remove all doubt. Easier to code, review and maintain. See this works even if `array` was declared earlier elsewhere and code was doing `array= malloc(MAX_str * sizeof *array);` – chux - Reinstate Monica Jan 08 '18 at 16:47
  • Do not updated your question with an answer. You can post your own answer to the question if you like. Post rolled back – chux - Reinstate Monica Jan 08 '18 at 17:05

2 Answers2

0

Code's primary problem is using an empty token string for subsequent calls of strtok(). Add as needed, characters to the 2nd token list

// array[++i] = strtok(NULL,"");
array[++i] = strtok(NULL," \t\n");

Improvement idea for getLine(): read and then allocate.

char* getLine(){
  char buf[MAX_str];
  if (fgets(buf, sizeof buf, stdin) == NULL) {
    return NULL;
  }
  return strdup(buf);
}

strdup() is common, although not in the C standard. Code example.

chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
0

Well, being stupid costs time... I have forgotten to reset i after while-loop in main()...

Is this code better? I want arr to be table of char pointers in amount of MAX_str. Is it declared well? With malloc would it be initialized like in answers above? To prevent array overflow I would compare i and value of MAX_str. Would it be ok?

#define MAX_str 100

char* getLine(){
  char buf[MAX_str];
  if (fgets(buf, sizeof buf, stdin) == NULL) {
    return NULL;
  }
  return strdup(buf);
}

int toArray(char* line, char* array[MAX_str]){
  int i=0;
  array[i] = strtok(line," \t\n");
  while(array[i]!=NULL){
    array[++i] = strtok(NULL," \t\n");
  }
  return i;
}

int main(){
  char* linia;
  char* arr[MAX_str];
  int i=0,end=0;

  while(!end){
    linia=getLine();
    assert(linia!=NULL);

    toArray(linia,arr);
    assert(arr!=NULL);

    while(arr[i]!=NULL){
      printf("%s\n",arr[i]);
      i++;
    }
  }
}
pjoter
  • 163
  • 2
  • 13