String array pointer segmentation fault when called multiple times in C

Question

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".

Jonathon Reinhart · Accepted Answer · 2014-03-25 02:04:22Z

2

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;
}

edited Mar 25, 2014 at 2:04

answered Mar 25, 2014 at 1:04

Jonathon Reinhart

138k38 gold badges266 silver badges347 bronze badges

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

5 Comments

Mariska Over a year ago

oh yes i made that silly mistake. i still got the same error. lemme update my post

Mariska Over a year ago

Regarding your static string version, as i mentioned in my post: "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" "

Jonathon Reinhart Over a year ago

Read the documentation for strcpy. It doesn't allocate any memory, and you're passing it a NULL pointer.

this Over a year ago

@JonathonReinhart You have some errors in foo. I was referring to the fact that you never check argv[1] which might be NULL.

Jonathon Reinhart Over a year ago

@self. D'oh. Copy and pasted the same bug I called out the OP for making, oops!

Collectives™ on Stack Overflow

String array pointer segmentation fault when called multiple times in C

1 Answer 1

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related