Question:
What's wrong with this C code?
Michael B
2012-01-17 09:04:07 UTC
include

struct bar {
int field1;
int field2;
int field3;
};

/* init_bar takes a pointer to a struct bar and initializes its fields */
void init_bar(struct bar *b, int val1, int val2, int val3) {
b->field1 = val1;
b->field2 = val2;
b->field3 = val3;
}


int buggy_func() {
struct bar *mybar;

init_bar(mybar, 42, 92, 86);

return mybar->field1;
}

int main() {
printf("The answer is %d\n",buggy_func());
return 0;
}

There is a segfault at the line:
return mybar->field1;

I think that this has to do with the that whatever happens in init_bar is local and the stack of init_bar disappears after it finishes. Even, if this is correct, please help me figure out how to describe this properly and refer to stacks and buffer overflow.
Three answers:
cja
2012-01-17 09:29:55 UTC
mybar is an uninitialized pointer; maybe its value is zero, or maybe some garbage value, but the segmentation fault you're getting is exactly what I would expect.



There are a few ways to fix your code:

1. in buggy_func:

      declare: struct bar mybar;

      call init_bar(&mybar, 42, 92, 86);

      return mybar.field1;



2. in buggy_func, declare and initialize: struct bar *mybar = malloc(sizeof(*mybar));



3. in init_bar: if (b == NULL) b = malloc(sizeof(*b));



I think option 1 is the easiest and best, I don't like functions allocating memory that will be the caller's responsibility to free. If you don't mind that, option 2 is ok if you want to use dynamic memory. Option 3 could work, but a non-zero uninitialized struct bar* would still be a problem.



EDIT: Re: "Shouldn't b->field1 = val1 within init_bar cause it to crash?" - sometimes you get lucky.
David Karr
2012-01-17 09:27:45 UTC
In "buggy_func", you've defined "mybar" as a pointer, but you haven't initialized it to anything, specifically, a pointer to a "bar" object. You have to allocate a bar object somewhere. Perhaps you should have just declared it as "struct bar mybar", and then passed "&mybar" to "init_bar", and then returned "mybar.field1".
2016-12-09 03:23:52 UTC
void WriteDay(char d[]); That line pronounces a function prototype. It we could the compiler understand that there is a function called WriteDay and what arguments ought to be handed to it. It does not tell the compiler how the WriteDay function is applied. you need to have the easily function someplace so as that the compiler is familiar with of what it fairly is meant to do.


This content was originally posted on Y! Answers, a Q&A website that shut down in 2021.
Loading...