1

I'm loosing my mind. I want to split string (char* text) with spaces and insert the string results into array and return this array. I have the following method in C

char *read_command(char *text)
{
    int index=0;
    char *res=NULL;
    char *command= (char*)malloc(strlen(text)+1);
    strcpy(command, text);
    char *tok = strtok(command, " ");

    while(tok!=NULL && index ==0)
    {
        res = (char*)realloc(res, sizeof(char)*(index+1));
        char *dup = (char*)malloc(strlen(tok)+1);
        strcpy(dup, tok);
        res[index++] = dup; //Error here
        tok = strtok(NULL, " ");
    }
    res[index++]='\0';

    return res;
}

from main method

char *input="read A B C";
char *command = read_command(input);

Thank you

user3132295
  • 288
  • 4
  • 23
  • `sizeof(char*)` ? , `&& index ==0` ??, `res[index++]='\0';` ??? – BLUEPIXY Dec 29 '13 at 12:50
  • So just to make it clear: do you want to concatenate the tokens? Or do you want to place them in an array of strings? Because for the latter, a `char *` is not good enough. Also, [don't cast the return value of `malloc()`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858#605858). –  Dec 29 '13 at 12:53
  • I'm realy new in C. if I send "read A B CCC" I want an array of strings so [0]=A [1]=B [2]=CCC thank you – user3132295 Dec 29 '13 at 12:59
  • Why are you using malloc+strcpy instead of `strdup`? – ThiefMaster Dec 29 '13 at 13:02

2 Answers2

1

You are using a wrong type to calculate the size in this call:

res = realloc(res, sizeof(char)*(index+1));

You need to use char*, not char, with sizeof, like this:

res = realloc(res, sizeof(char*)*(index+1));

Since your code returns a pointer to C strings (represented as char*) the return type should be char**.

You need to remove the index == 0 condition from the while loop, otherwise it wouldn't go past the initial iteration.

This assignment

res[index++]='\0';

should be

res[index++]=NULL;

You also need to call free(command) before returning the results to the caller. Finally, you should not cast results of malloc in C.

Here is your code after the fixes above:

char **read_command(char *text) {
    int index=0;
    char **res=NULL;
    char *command= malloc(strlen(text)+1);
    strcpy(command, text);
    char *tok = strtok(command, " ");
    while(tok!=NULL) {
        res = realloc(res, sizeof(char*)*(index+1));
        char *dup = malloc(strlen(tok)+1);
        strcpy(dup, tok);
        res[index++] = dup;
        tok = strtok(NULL, " ");
    }
    // Need space to store the "terminating" NULL
    // Thanks, BLUEPIXY, for pointing this out.
    res = realloc(res, sizeof(char*)*(index+1));
    res[index]=NULL;
    free(command);
    return res;
}

Demo on ideone.

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • Thank you very much it's working. Can you please explain why I shouldn't cast the malloc, realloc result? In the course I'm studing now I have been told that I must cast the malloc result? – user3132295 Dec 30 '13 at 07:09
  • @user3132295 Casting `malloc` is required in C++, but in C it is optional. Here is [a link to an answer explaining the reasons why it shouldn't be done](http://stackoverflow.com/a/605858/335858). – Sergey Kalinichenko Dec 30 '13 at 11:06
0
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char **read_command(const char *text){
    int index=0;
    char **res=NULL;
    char *command= malloc(strlen(text)+1);
    strcpy(command, text+strspn(text, " \t\n"));//strspn for skip space from top
    char *tok = strtok(command, " ");

    res = realloc(res, sizeof(char*)*(index+1));
    while(tok!=NULL){
        res[index++] = tok;
        res = realloc(res, sizeof(char*)*(index+1));
        tok = strtok(NULL, " ");
    }
    res[index++]=NULL;

    return res;
}

int main(void){
    char *input="read A B C";
    char **command = read_command(input);
    int i;
    for(i=0;command[i]!=NULL;++i){
        printf("s[%d]=%s\n", i, command[i]);
    }
    free(command[0]);//for free command of read_command
    free(command);//for free res of read_command,,
    return 0;
}
BLUEPIXY
  • 39,699
  • 7
  • 33
  • 70