CODEWITHSUNDEEP
csegmentation-fault
WhiteSword
WhiteSword

Reputation: 101

Segmentation fault occured on passing a pointer to a char array into a function

This piece of code was working fine, until I modified it.

int main(int argc, char** argv)
{
    char command[1024];
    gets(command);
    char *delim = " \t\f";
    char **tokens;
    int i=0;
    /* Extracting tokens from command string */
    for(tokens[i] = strtok(command, delim); tokens[i] != NULL; tokens[i] = strtok(NULL, delim))
    {
       i++;
    }
    return 0;
}

And here's the code that crashed with a Segmentation Fault on line for(tokens[i] = strtok(command, delim); tokens[i] != NULL; tokens[i] = strtok(NULL, delim))

Code:

void commandProcess(char command[])
{
    char *delim = " \t\f";
    char **tokens;
    int i=0;
    /* Extracting tokens from command string */
    for(tokens[i] = strtok(command, delim); tokens[i] != NULL; tokens[i] = strtok(NULL, delim))
    {
       i++;
    }
}

int main(int argc, char** argv)
{
    char command[1024];
    gets(command);
    process(command);
    return 0;
}

I know that char command[] decays to a pointer link, and also that strtok() modifies its first parameter and in my case it might be an undefined behaviour. link

So, could anybody please provide me some other way around, so that I can work out with the same function signature and still avoid the problem?

This seems a trivial problem, but I cannot make through it. :\ I even tried this,

void commandProcess(char command1[])
{
    char command[1024];
    int length = strlen(command1);
    strncat(command, command1, length);
    command[length] = '\0';

    char *delim = " \t\f";
    char **tokens;
    int i=0;

    /* Extracting tokens from command string */
    for(tokens[i] = strtok(temp_command, delim); tokens[i] != NULL; tokens[i] = strtok(NULL, delim))
    {
       i++;
    }

but then again crashes on the line

for(tokens[i] = strtok(temp_command, delim); tokens[i] != NULL; tokens[i] = strtok(NULL, delim))

I think that delim & tokens are not the culprits because the program without commandProcess() worked fine with them.

Upvotes: 1

Views: 591

Answers (2)

Steve Summit
Steve Summit

Reputation: 48033

As other comments and answers have pointed out, you need to allocate space for the token pointers you collect.

Here's a fixed version, which also uses fgets() instead of the obsolete and dangerous gets(). Normally, unfortunately, fgets is a bit of a nuisance, because it leaves the trailing \n in the buffer. Here, however, that's trivial to account for, as we can simply add '\n' to the delim variable telling strtok what to split on.

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

#define MAXTOKS 100

void commandProcess(char command[])
{
    char *delim = " \t\f\r\n";
    char *tokens[MAXTOKS+1];
    int i=0;
    /* Extracting tokens from command string */
    for(tokens[i] = strtok(command, delim); tokens[i] != NULL; tokens[i] = strtok(NULL, delim))
    {
       i++;
       if(i > MAXTOKS)
       {
          fprintf(stderr, "too many tokens\n");
          exit(1);   
       }
    }
    for(i = 0; tokens[i] != NULL; i++) printf("%d: \"%s\"\n", i, tokens[i]);
}

int main(int argc, char** argv)
{
    char command[1024];
    fgets(command, sizeof(command), stdin);
    commandProcess(command);
    return 0;
}

Upvotes: 1

dbush
dbush

Reputation: 225007

You write to tokens[i], but tokens is never initialized. Dereferencing an uninitialized pointer invokes undefined behavior.

This means the behavior of the code is unpredictable. It may crash, it may show strange results, or it may appear to work correctly. Also, making a seemingly unrelated code change can change how undefined behavior manifests. This is what happened in your case.

To fix this, you need to allocate space to tokens. In this case, the simplest thing to do is make a fixed size array of pointers:

char *tokens[1024];

You can also malloc the memory so you don't have a large array on the stack:

char **tokens = malloc(sizeof(char *) * 1024);

Since command is of length 1024, the number of tokens shouldn't be any larger.

Upvotes: 1

Related Questions