user3332718
user3332718

Reputation: 25

Segmentation fault with array of strings C

I have made a program that breaks a string into tokens separated by space and then copies each separate string into an array of strings.

Program works fine until it reaches for-loop and after successfully adding the first string into the array and printing it, program crashes. I debugged it and found that

args[i] = malloc((strlen(comm_str) + 1) * sizeof(char));

returns SEGFAULT then performing loop for the second time. Also call stack prints out the following:

Address: 75F943F9, Function: strlen(), File: C:\Windows\syswow64\msvcrt.dll.`

I tried to correct program myself, but with no result. I thought at first that loop tries to access out of bound region, but I think I have malloc'd everything correctly.

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

int main(){

    char **args = NULL;
    char input[] = "cmdline -s 20 -r -t parameter -p 20 filename";

    int num = 0;
    char *comm_str = strtok(input, " ");                            /*Tokens command line*/

    while(comm_str != NULL){                                        /*Counts number of tokens*/
        num++;
        printf("%s %d\n", comm_str, strlen(comm_str));
        comm_str = strtok(NULL, " ");
    }
    printf("\n");

    args = malloc(num * sizeof(char*));                          /*Allocates memory for the first string entry*/
    if(!args){
        return 0;
    }

    comm_str = strtok(input, " ");                                  /*Starts tokening from beginning*/
    for(int i = 0; i < num; i++){
        args[i] = malloc((strlen(comm_str) + 1) * sizeof(char));    /*Allocates memory for strings*/
        if(!args[i]){
            for(int b = 0; b < i; b++){
                free(args[b]);
            }
            free(args);
            return 0;
        }
        strcpy(args[i], comm_str);
        printf("%s\n", args[i]);
        comm_str = strtok(NULL, " ");
    }

    printf("%d\n", num);

}

Upvotes: 2

Views: 861

Answers (4)

Fiddling Bits
Fiddling Bits

Reputation: 8861

strtok replaces each ' ' with a NULL in your case. Consider the following:

char input[] = "cmdline -s 20 -r -t parameter -p 20 filename";
size_t inputSize = strlen(input);

for(int i = 0; i < inputSize; i++)
    printf("%02X ", input[i]);
printf("\n\n");

char *comm_str = strtok(input, " "); 
while(comm_str != NULL)
    comm_str = strtok(NULL, " ");

for(int i = 0; i < inputSize; i++)
    printf("%02X ", input[i]);
printf("\n\n");

Output:

63 6D 64 6C 69 6E 65 20 2D 73 20 32 30 20 2D 72 20 2D 74 20 70 61 72 61 6D 65 74 65 72 20 2D 70 20 32 30 20 66 69 6C 65 6E 61 6D 65 

63 6D 64 6C 69 6E 65 00 2D 73 00 32 30 00 2D 72 00 2D 74 00 70 61 72 61 6D 65 74 65 72 00 2D 70 00 32 30 00 66 69 6C 65 6E 61 6D 65

Upvotes: 0

Cantfindname
Cantfindname

Reputation: 2148

According to the man pages for strtok: "Be cautious when using these functions. If you do use them, note that: These functions modify their first argument. These functions cannot be used on constant strings."

So, the first time you parse to find num, you modify your input string, and as such the second time you parse garbage, which causes the SEGFAULT. Also, you shouldn't use the constant string :

char input[] = "cmdline -s 20 -r -t parameter -p 20 filename";

Upvotes: 1

abligh
abligh

Reputation: 25189

The first problem is that you have not included the relevant function prototypes. This will make it assume everything is an integer. See below:

$ gcc -Wall x.c -o x
x.c: In function ‘main’:
x.c:9:5: warning: implicit declaration of function ‘strtok’ [-Wimplicit-function-declaration]
x.c:9:22: warning: initialization makes pointer from integer without a cast [enabled by default]
x.c:13:9: warning: implicit declaration of function ‘strlen’ [-Wimplicit-function-declaration]
x.c:13:37: warning: incompatible implicit declaration of built-in function ‘strlen’ [enabled by default]
x.c:13:9: warning: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘long unsigned int’ [-Wformat]
x.c:14:18: warning: assignment makes pointer from integer without a cast [enabled by default]
x.c:18:5: warning: implicit declaration of function ‘malloc’ [-Wimplicit-function-declaration]
x.c:18:12: warning: incompatible implicit declaration of built-in function ‘malloc’ [enabled by default]
x.c:23:14: warning: assignment makes pointer from integer without a cast [enabled by default]
x.c:24:5: error: ‘for’ loop initial declarations are only allowed in C99 mode
x.c:24:5: note: use option -std=c99 or -std=gnu99 to compile your code
x.c:25:27: warning: incompatible implicit declaration of built-in function ‘strlen’ [enabled by default]
x.c:27:13: error: ‘for’ loop initial declarations are only allowed in C99 mode
x.c:28:17: warning: implicit declaration of function ‘free’ [-Wimplicit-function-declaration]
x.c:28:17: warning: incompatible implicit declaration of built-in function ‘free’ [enabled by default]
x.c:30:13: warning: incompatible implicit declaration of built-in function ‘free’ [enabled by default]
x.c:33:9: warning: implicit declaration of function ‘strcpy’ [-Wimplicit-function-declaration]
x.c:33:9: warning: incompatible implicit declaration of built-in function ‘strcpy’ [enabled by default]
x.c:35:18: warning: assignment makes pointer from integer without a cast [enabled by default]
x.c:40:1: warning: control reaches end of non-void function [-Wreturn-type]

I suggest you fix that before proceeding.

Upvotes: 0

Karoly Horvath
Karoly Horvath

Reputation: 96306

strtok is, as you know, altering the string.

After calculating the number of words, the string will contain a single word. Hence the next strtok will return NULL.

Calculate the number of arguments in a non-destructive way, or make a copy of the string.

Upvotes: 4

Related Questions