Why does this struct that contains a pointer to itself not work properly?

Question

#include <stdio.h>
#include <stdlib.h>
typedef struct minion
{
    int home;
    struct minion *father;  
} Minion;
main()
{
    Minion *dummy = (Minion *)malloc(sizeof(Minion));
    dummy->father = &dummy;
    dummy->home = 0;
    printf("%d %d %d", &dummy, dummy->father, (dummy->father)->father);
}

Why is the value of (dummy->father)->father different from the others?

The output is:

2424368 2424368 43

You re assigning "pointer to pointer" to a "pointer": `dummy->father = &dummy;`. This should not compile in c++. — , Dec 10 '15 at 19:44
@Slava How do you mean..? The only downside to `typedef` a `struct` is that you're hiding the fact that it's a struct when it's used in the program. — Nick Zuber, Dec 10 '15 at 19:52
The format specifier to format a pointer is _not_ `%d`. It is `%p` (and I think it is strictly speaking necessary to convert the argument pointers to `void*` or `void const*`). — Dietmar Kühl, Dec 10 '15 at 19:53
@NickZ I mean try to forward declare `Minion` to use pointer or reference without including header. Especially when struct is anonymous. — Slava, Dec 10 '15 at 19:53
@Arkadiy unfortunately, at least one popular compiler (gcc) has a bug in that it fails to give an error for incompatible type conversions, in its default mode of operation . This misleads people. — M.M, Dec 10 '15 at 19:56
yeah this code is in c!i tagged c++ because of my distraction — E.Pajouheshgar, Dec 10 '15 at 19:58
@KeithThompson it is, when you have to use C library in C++ and want to make a wrapper you have to use `void *` because of this creepiness. — Slava, Dec 10 '15 at 20:03
[Don't cast the result of `malloc()` in C code.](http://stackoverflow.com/q/605845/315052) — jxh, Dec 10 '15 at 20:12
@Slava: I don't understand. Do you have an example? Typedefs for structures are as valid (and as unnecessary) in C++ as they are in C. — Keith Thompson, Dec 10 '15 at 20:13
@KeithThompson it is pretty common in C library to write `typedef struct { ... } crap;` and then use `crap *` as handles. I do not want to expose C header to the rest of C++ code, so I made thin wrapper to avoid include and I cannot forward declare `crap` to use `crap *` without include. So I have to use `void *` which is unsafe. — Slava, Dec 10 '15 at 20:16
@Slava: `typedef struct crap crap;` works just fine in a header file if the user of the library only needs to use `crap *`. — jxh, Dec 10 '15 at 20:21
@jxh yea right, now try to include header that has original `typedef struct { ... } crap;` - http://ideone.com/gbD53f — Slava, Dec 10 '15 at 20:59
@Slava: Of course a syntax error is a syntax error. There is a difference between `struct crap {}` and an anonymous `struct {}`. The header file does not define the structure, but the library implementation provides a real definition for `struct crap`. http://ideone.com/1kgEB5 — jxh, Dec 10 '15 at 21:11
@jxh Ias you can see that it does not work with anonymous struct what I actually mention in my post, so it does not work and I cannot change header of 3rd party library to make structs non anonymous. — Slava, Dec 10 '15 at 21:35
@Slava: You do not gain anything from using `void *` as the handle type other than avoiding the work of creating a proper type-safe thin wrapper. My comments indicate it is possible to create a type-safe wrapper, even if the wrapper has to inherit from the anonymous type in its implementation. — jxh, Dec 10 '15 at 21:38
@jxh your example uses non anonymous structure so your comments do not indicate that it is possible. — Slava, Dec 10 '15 at 21:54
@Slava: I am simply leaving the work of actually writing a type safe wrapper in your hands. If you are unclear as to how to proceed, feel free to post a question. — jxh, Dec 10 '15 at 21:58

score 2 · Accepted Answer · answered Dec 10 '15 at 20:12

main()

This should be int main(void).

{
    Minion *dummy = (Minion *)malloc(sizeof(Minion));

This is more clearly and reliably written as:

Minion *dummy = malloc(sizeof *dummy);

...

    dummy->father = &dummy;
    dummy->home = 0;

That's ok.

    printf("%d %d %d", &dummy, dummy->father, (dummy->father)->father);

You have several problems here.

dummy is a pointer object. &dummy is the address of that pointer object. The pointer object is allocated on the stack, and there's no particular reason to care about its address.

If you want to print the address of the Minion object you allocated, change &dummy to just dummy; you want the value of the pointer object.

%d is used only for printing values of type int, not pointers. To print a pointer value, use %p, which expects an argument of type void*, so in general you'll have to convert the pointer value to void* before printing it. That line should look like this:

printf("%p %p %p\n",
       (void*)dummy,
       (void*)dummy->father,
       (void*)dummy->father->father);

Note that I've added a \n to the end of the line so it will print properly. I've also removed an unnecessary set of parentheses.

One more minor point, not relevant to your problem. It's common to define a typedef for each struct type -- but it's not necessary. My own personal preference is to omit the typedef and refer to the structure type by its original name -- in this case, struct minion. It's simpler (since it's easier to refer to the type before it's been completely defined). It's a little more typing, but IMHO the code is clearer if struct types are visibly structures. (It makes sense to use a typedef if you want the type to be opaque, so that code using the type doesn't know that it's a structure. You're not doing that here.)

On the other hand, plenty of C programmers prefer to use typedefs for structures because it provides a name for the type that's a single identifier. Pick a style and be consistent.

Olivier Pellier-Cuit · Answer 2 · 2015-12-10T20:06:15.543

0

#include <stdio.h>
#include <stdlib.h>
typedef struct minion
{
    int home;
    struct minion *father;  
} Minion;
main()
{
    Minion *dummy = (Minion *)malloc(sizeof(Minion));
    /* dummy->father = &dummy <== already a pointer */
    dummy->father = dummy;
    dummy->home = 0;
    printf("%p %p %p", dummy, dummy->father, (dummy->father)->father);
}

edited Dec 10 '15 at 20:06

answered Dec 10 '15 at 19:54

Olivier Pellier-Cuit

1,269
9
15

yeah its better to use %p but it doesnt solve the problem. – E.Pajouheshgar Dec 10 '15 at 20:02
@E.Pajouheshgar removing & from dummy does and he already showed that to you. – Slava Dec 10 '15 at 20:04
`%p` expects an argument of type `void*`. See my answer. – Keith Thompson Dec 10 '15 at 20:12

Why does this struct that contains a pointer to itself not work properly?

2 Answers2