Mariska
Mariska

Reputation: 1953

String array pointer segmentation fault when called multiple times in C

I have a very simple program that checks the user argument and prints something. Here it is:

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

const char * foo(char * input){
    char *result = NULL;
    strcpy(result, "{ ");
    if (strcmp(input, "kittycat") == 0){
        strcat(result, "JACKPOT!");
    }
    else{
        strcat(result, "Nothing");  
    }
    strcat(result, " }");
    return result;
}

int main(int argc, char *argv[]){
    printf("%s\n", foo(argv[1]));
    printf("%s\n", foo(argv[1]));
    printf("%s\n", foo(argv[1]));
    return 0;
}

In main(), if I print printf("%s\n", foo(argv[1])); just once, the program runs without errors. However, if I print it three times as shown above, I get "Segmentation fault: 11". Any ideas? I know I can simplify foo and avoid the use of "char *result", but I would like to understand what's wrong with my usage of "char *result".

Upvotes: 1

Views: 82

Answers (1)

Jonathon Reinhart
Jonathon Reinhart

Reputation: 137398

const char * foo(char * input) {
    char *result;
    strcpy(result, "{ ");    // 'result' used uninitialized - undefined behavior 

result is uninitialized. Pay attention to compiler warnings.

Also, I'm assuming you want to be checking input, not result here:

if (strcmp(result, "kittycat") == 0) {

This version returns static strings:

const char *foo(char *input) {
    if (strcmp(input, "kittycat") == 0)
        return "{ JACKPOT! }";
    return "{ Nothing }";
}

This version returns mallocd strings, which you need to free:

#define MAX_FOO_RESULT    20

const char *foo(char *input) {
    char *result = malloc(MAX_FOO_RESULT+1);
    if (!result) return NULL;
    result[0] = '\0';

    strncat(result, "{ ", MAX_FOO_RESULT);

    if (strcmp(input, "kittycat") == 0)
        strncat(result, "JACKPOT!", MAX_FOO_RESULT);
    else
        strncat(return, "Nothing", MAX_FOO_RESULT);

    strncat(result, " }", MAX_FOO_RESULT);

    return result;
}

int main(int argc, char *argv[]){
    const char* res;

    if (argc < 2) return 1;

    // memory leak - result of foo is leaked 
    printf("%s\n", foo(argv[1]));     

    // fixed leak
    res = foo(argv[1]);
    if (res) {
        printf("%s\n", res);
        free(res);
    }

    return 0;
}

Upvotes: 2

Related Questions