Reputation: 1953
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
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 malloc
d 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