Why does this program work fine on my computer, but cause an overflow on another computer?

Question

I'm teaching a friend C. We were working with structs and pointers and I gave him a program to try out on his computer. We were going to deconstruct the program line by line so he could understand how structs and pointers worked together. On my end, I get this result:

Value of a in astr is 5
Value of b in astr is 5.550000
Value of c in astr is 77
Value of d in astr is 888.888800

On his computer, the program mostly worked except for the last value of astr->d which printed out some very large negative number. So my question is, why does this happen on his computer, but work fine on mine? below is the offending code:

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

int main(){
    struct a_struct{
        int a;
        float b;
        int c;
        double d;
    };

    struct a_struct* astr;

    astr = (struct a_struct*)malloc(sizeof(astr));

    astr->a = 5;
    astr->b = 5.55;
    astr->c = 77;
    astr->d = 888.8888;

    printf("Value of a in astr is %d\n", astr->a);
    printf("Value of b in astr is %f\n", astr->b);
    printf("Value of c in astr is %d\n", astr->c);
    printf("Value of d in astr is %lf\n", astr->d);

    return 0;
}

I think your `sizeof` is incorrect. I would recommend this: `astr = (struct a_struct*)malloc(sizeof(struct a_struct));` — George Bou, May 29 '16 at 20:17
No, `astr = malloc (sizeof *astr);` See: [**Do I cast the result of malloc?**](http://stackoverflow.com/q/605845/995714) for thorough explanation. — David C. Rankin, May 29 '16 at 20:25
Please don't try teaching C if you are not good at it yourself. — too honest for this site, May 29 '16 at 20:43
God forbid I make a mistake. I'm human. And they say that you learn more as a teacher, than as a student. I'm not a professional programmer, I know. I program as a hobby. — NiaKitty, May 29 '16 at 20:52

Keith Thompson · Accepted Answer · 2016-05-29T21:56:56.910

10

You have at least two problems.

First, your malloc call is incorrect.

astr = (struct a_struct*)malloc(sizeof(astr));

astr is a pointer, so sizeof(astr) is the size of a pointer. You want to allocate enough memory to hold an object of type struct astruct.

astr = (struct a_struct*)malloc(sizeof (struct a_struct)));

Or, more simply and robustly:

astr = malloc(sizeof *astr);

(The argument to sizeof is not evaluated, so sizeof *astr gives you the size of what astr points to without trying to dereference it. The cast is unnecessary because malloc returns a void*, which is implicitly converted to the required pointer type.)

Second, you're using ~~the wrong format~~ not quite the right format to print astr->d. The %f format works for both float and double arguments (because float arguments to printf are promoted to double). The correct format for a long double argument is %Lf. Just use %f for both astr->b and astr->d. (Starting with C99 "%lf" is equivalent to %f, but it's better just to use %f'.)

Third (ok, I miscounted), you should check whether the malloc call succeeded by comparing the result to NULL. It's not likely to fail for such a small allocation, but it's a very good habit to check consistently. If it fails, you can just abort the program with an error message (for larger programs some more sophisticated error handling might be called for).

edited May 29 '16 at 21:56

answered May 29 '16 at 20:18

Keith Thompson

254,901
44
429
631

I can't upvote again, but unconciously I tried to, because of "*Third*". – Iharob Al Asimi May 29 '16 at 20:19
Your second position is not an issue actually, because of standard promotions. The "ell" is just redundant. – too honest for this site May 29 '16 at 20:46
Thanks a ton. Yeah, I definitely should have done "sizeof struct a_struct" instead. I normally would have done that but hey, everyone makes mistakes. – NiaKitty May 29 '16 at 20:53
@Olaf: The `%lf` format is ok in C99 and later -- not because of standard promotions, but because the standard says that `l` has no effect on a following `f` conversion. In C90, that rule didn't exist, so the behavior was undefined. I'll update my answer. – Keith Thompson May 29 '16 at 21:55
@NiaKitty: Writing `astr = malloc(sizeof *astr);` is more robust than mentioning the type explicitly. You don't have to update it if the type that `astr` points to changes. `astr = malloc(sizeof (struct a_struct));` is perfectly valid, but too easy to break as the code is maintained. – Keith Thompson May 29 '16 at 21:59
@KeithThompson: Sorry, I was unclear. `%f` works because of the standard arithmetic promotions `float` to `double`. This is an asymmetry to `scanf`. `%lf` for `double`, to makes that identical to `scanf`. Whether one uses `%f` or `%lf` is a personal decission and makes not really a difference. And the C tag implies standard C, so both are correct. Not sure where you see a `long double` argument, though. – too honest for this site May 29 '16 at 23:23
@NiaKitty: Please read the standard (or a good book). If using with types, you need the function-form of `sizeof`. – too honest for this site May 29 '16 at 23:26
@Olaf: I incorrectly wrote `sizeof struct a_strict` in my answer, so that's my fault. I later corrected it to add the parentheses. (A compiler would catch the missing parens *very* quickly). BTW, `sizeof (` *type-name* `)` is not "the function-form". The parentheses do not correspond to the parentheses in a function call. They're just part of the syntax of `sizeof`.) – Keith Thompson May 30 '16 at 00:36
@Olaf: `%lf` was added in C99, a change that made existing incorrect code valid. Using `%f` makes your code potentially portable even to older implementations. – Keith Thompson May 30 '16 at 00:37
@KeithThompson: C99 has some subtle incompatibilities (resp. more exactly defined behaviour), so portability can be more of a problem anyway. In general, I strongly support moving away from ancient C90. It is just a burden and should be burried deep. It definitively **is** a matter of personal preference. As I don't use this part of the stdlib anyway in my project, I actually don't care much. – too honest for this site May 30 '16 at 03:22
@Olaf: Using `"%f"` for `float` and `double` arguments to `printf` is perfectly portable. C99 did add permission to use `"%lf"`, but there's no particular reason to take advantage of it. – Keith Thompson May 30 '16 at 04:10
Even before C99, `%lf` was a very common non-standard extension that most compilers supported. – Lundin May 30 '16 at 06:43

score 1 · Answer 2 · answered May 29 '16 at 20:17

1

This is because the program is allocating memory incorrectly. The below statement actually assign memory equal to the size of the pointer, which could be 32 bit or 64 bit depending on the OS architecture.

struct a_struct* astr;
astr = (struct a_struct*)malloc(sizeof(astr));

Instead you should do:

struct a_struct* astr;
astr = (struct a_struct*)malloc(sizeof(struct a_struct));

answered May 29 '16 at 20:17

Piyg

224
2
10

Looks like George Bou and I both were typing the same answer at the same time. George beat me by 35 seconds. :) – Piyg May 29 '16 at 20:19
Honestly I don't like your answer, it's correct nevertheless so downvoting is not necessary. – Iharob Al Asimi May 29 '16 at 20:21
1

But you provided a more detailed answer. Well done! :) – George Bou May 29 '16 at 20:21
iharob, agree that it is not as good and complete as the answer provided by Keith. – Piyg May 29 '16 at 20:24

user3078414 · Answer 3 · 2016-05-29T20:44:02.333

0

If you allocate memory dynamically you should take care to allocate memory for the object to hold your data, not only a pointer to the first member of something you haven't allocated. "The other way around" would be to explicitly declare the struct as variable first, and then the pointer to point at its address:

struct a_struct astrV;
struct a_struct *astr = &astrV;

edited May 29 '16 at 20:44

answered May 29 '16 at 20:38

user3078414

1,942
2
16
24

Why does this program work fine on my computer, but cause an overflow on another computer?

3 Answers3