Splitting C char array into words

Question

I am trying to split a given char array into separate strings. I am doing this by putting the address of each word into an array, and then getting the string from the address to print.

So I have updated my code but now the program freezes after printing the numArgs but before "test2." I don't understand why.

----------------old code-----------------------   
char* parseArgs(char* comPtr){
    char *args[100] = {0};
    char *token;
    int i = 0;
    token = strtok(comPtr, " ");
    while(token != NULL){
        args[i] = malloc(100);
        args[i] = &token;
        token = strtok(NULL, " ");
    }
    return *args;
}

char* args = parseArgs(comPtr);
int i = 0;
while(i < numArgs){
    printf("arg%d: %s\n",i,&args[i]);
    i++;
}
-----------------------end old code--------------------

------------------new code------------------------
int countArgs(char* comPtr){
    char *token;
    int i = 0;
    token = strtok(comPtr, " ");
    while(token != NULL){
        i++;
        token = strtok(NULL, " ");
    }
    return i;
}

char** parseArgs(char* comPtr){
    printf("test1");
    char** args = calloc(100, sizeof(char*));
    char* token;
    int i = 0;
    while(token = strtok(comPtr, " ")){
        args[i] = token;
    }
    printf("test2");
    return args;
}

printf("ComPtr: %s\n",comPtr);
char* path = "/bin/";
//int pid = fork(); //pid always 0 so using pid = 1 to test
//printf("PID:%d",pid);
int pid = 1;
printf("PID:%d",pid);
if(pid != 0){
    int numArgs = countArgs(comPtr);
    printf("test1");
    printf("NumArgs: %d\n",numArgs);
    printf("test2");
    char** args = parseArgs(comPtr);
    int i = 0;
    printf("test3");
    while(i < numArgs){
        printf("arg%d: %s\n",i,args[i]);
        printf("test4");
        i++;
    }
}
else{
    //waitpid();
}

Umm: args[i] = malloc(100); args[i] = &token; You just leaked memory it two short lines. — WhozCraig
– WhozCraig, Commented Mar 8, 2013 at 6:28
You should't return local variable args from function parseArgs — Rohan
– Rohan, Commented Mar 8, 2013 at 6:28
FYI strtok will modify its first argument. The spaces in your string are being replaced in countArgs with a \0 then you are trying to parse the spaces (which are no longer there) in parseArgs. Also, i is never incremented in parseArgs. — Kludas
– Kludas, Commented Mar 8, 2013 at 9:29
Your code doesn't compile. How are we supposed to reproduce the error if we can't compile the code? Please show us a minimal, compilable testcase. — autistic
– autistic, Commented Mar 8, 2013 at 9:36

Tony Delroy · Accepted Answer · 2013-03-08 06:36:47Z

2

You've lost track of where your memory is, your pointers are pointing etc.. If you want to return the list of pointers to tokens, you need something like this:

char** parseArgs(char* comPtr){
    char** p_args = calloc(100, sizeof(char*);
    int i = 0;
    char* token;
    while (token = strtok(comPtr, " "))
        p_args[i] = token;
    return p_args;
}

char** p_args = parseArgs(comPtr);
int i = 0;
while(i < numArgs)
{
    printf("arg%d: %s\n",i,p_args[i]);
    i++;
}
free(p_args);

I haven't tested it, but it should point you in the right direction. Have a careful think about how it differs from your program, and use a debugger and/or printf() statements in the code to print out addresses and see how it works (or debug it if necessary).

answered Mar 8, 2013 at 6:36

Tony Delroy

107k16 gold badges188 silver badges265 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

Anish Ramaswamy Over a year ago

To be pedantic, wouldn't it be additionally fail-safe if you set p_args to NULL after freeing it?

Trent W Over a year ago

Thanks but now I can't even test this due to the program freezing

Tony Delroy Over a year ago

@AnishRam: IMHO, there's no overarching best practice for this... can make sense if you're worried about someone using or freeing it again (e.g. in a long complex function body), but you might know that's impossible (e.g. the next source line closes the scope in which p_args was created on the stack). Personally, I very rarely find it useful to NULL pointers, but then I program C++ these days where freeing is often hidden in destructors and the issue rarely exists....

Tony Delroy Over a year ago

@TrentWorkman: if a program freezes, then as per my answer you should use a debugger or add printf() statements to identify the line it freezes on. You'll never make a programmer if you give up so easily. I'm happy to point you in the right direction, but no chance I'm holding your hand all the way. If you get stuck, ask a specific problem ala "it gets to the line that says xyz and this pointer's this and it does that and I expected it to do something else, I tried this but it did that... what's going on and how do I fix it?".

Trent W Over a year ago

Thank you but I did tell which lines it froze on. I totally understand about holding my hand but I just really don't understand why it would freeze where it does. It freezes between two printf () statements and that would seem to me as something wrong with printf

|

Sankar Mani · Accepted Answer · 2013-03-08 06:30:46Z

0

Declare the pointer array 'char *args[100]' as global variable. In your program your are allocating memory to the local pointer and its life is within the function. so at the end of the function your pointer variable scope ends. Here there is memory leak too.

answered Mar 8, 2013 at 6:30

Sankar Mani

1783 bronze badges

Comments

Daniel Fischer · Accepted Answer · 2013-03-08 12:16:43Z

0

The freeze is due to

int i = 0;
while(token = strtok(comPtr, " ")){
    args[i] = token;
}

where you repeatedly - in an infinite loop - find the first token in comPtr, token becomes &comPtr[0] in each iteration (unless the string starts with spaces), and that is assigned to args[i].

After the first call, all calls to strtok that shall find further tokens in the same string - if any - should have a NULL first argument.

Also, you should probably increment i in the loop, since presumably you don't want to overwrite args[0] with each new token.

answered Mar 8, 2013 at 12:16

Daniel Fischer

184k19 gold badges319 silver badges436 bronze badges

Collectives™ on Stack Overflow

Splitting C char array into words

3 Answers 3

6 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

6 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related