3

This is my first post. I'm very confused with the C's pointer and its relation to struct. I've searched for more information but can't really conclude them. For example given this struct definition

typedef struct node
{
     int info;
     struct node *next;
}NODE;

Then what's the differences and effects of these four declarations;

1. NODE *node1 = malloc (sizeof(NODE));
2. NODE *node1 = (struct node *) malloc (sizeof(NODE));
3. NODE *node1 = (struct node *) malloc (sizeof(NODE *));
4. NODE *node1 = malloc (sizeof(NODE *));

Thanks in advance.

James Real
  • 87
  • 4
  • 1
    Your problem is not with pointers or structs, it's with `C++` and `sizeof` – EOF Nov 18 '16 at 19:28
  • In C you do not cast the return value from malloc - see http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc – Ed Heal Nov 18 '16 at 19:29
  • 4
    @EOF - Who mentioned C++? – Ed Heal Nov 18 '16 at 19:29
  • If you remove the unnecessary casts, you are left with only 2 examples: memory for a `struct`, and memory for a `struct ` pointer. – Weather Vane Nov 18 '16 at 19:30
  • You need (1) - A pointer to a node - hence allocate memory for a node – Ed Heal Nov 18 '16 at 19:32
  • @WeatherVane But I've seen some C examples using such casts and in fact uses both. – James Real Nov 18 '16 at 19:33
  • Example 3 and 4 are pointless, because you just allocate space for a _pointer_ and not for the struct. 1 and 2 are more or less the same, the `(struct node *)` is not necessary (some will say it's wrong). – Jabberwocky Nov 18 '16 at 19:33
  • A pointer is simply an integer type that holds a memory address, and is the same size regardless of what it points to; generally, it'll be large enough to address all of a system's memory. `sizeof(NODE *) == sizeof(void *)`, for example. `struct node` will be `sizeof(int) + sizeof(struct node *)`, potentially with additional padding. Due to this, #3 and #4 will be too small. – Justin Time - Reinstate Monica Nov 18 '16 at 19:34
  • @EdHeal Ok. But how is it any different than 2? Is it technically incorrect or something? – James Real Nov 18 '16 at 19:34
  • @JamesReal there are many C examples using such casts, that does not make them right: please follow the link from Ed Heal, second comment. – Weather Vane Nov 18 '16 at 19:36
  • Please read the link – Ed Heal Nov 18 '16 at 19:36
  • @EdHeal Those useless casts look like somebody learned C from a C++ environment. – EOF Nov 18 '16 at 19:37
  • @JustinTime - _A pointer is simply an integer type .._ - this is incorrect – Ed Heal Nov 18 '16 at 19:37
  • Thanks for the pointers guys. – James Real Nov 18 '16 at 19:47
  • 1
    @JustinTime there are exceptions to pointers always being the same size, here is one [SO question](http://stackoverflow.com/questions/3520059/does-the-size-of-pointers-vary-in-c) with further links. – Weather Vane Nov 18 '16 at 19:47
  • @EdHeal I mean mechanically (stores addresses as integers, compatible with integer types `ptrdiff_t`, `intptr_t`, and `uintptr_t`), not semantically (counted in the "integer types" arithmetic type group, and subject to its rules). I probably should've actually _said_ that, though, especially considering C actually has a group of types explicitly called "integer types", so it's my bad. – Justin Time - Reinstate Monica Nov 18 '16 at 19:59
  • @WeatherVane I completely forgot about all that near/far/huge/tiny stuff (probably because I've never actually had any experience with them, and only have very limited second-hand knowledge of how that stuff works), and somehow also forgot both that there are architectures where pointers aren't necessarily the same size (like ones that aren't byte-addressable), and that function pointers don't play nicely with regular pointers. I guess that's my bad, too. – Justin Time - Reinstate Monica Nov 18 '16 at 20:06

3 Answers3

6

These are equivalent and allocate a block of memory the same size as a NODE struct.

1. NODE *node1 = malloc (sizeof(NODE));
2. NODE *node1 = (struct node *) malloc (sizeof(NODE));

These are just wrong

3. NODE *node1 = (struct node *) malloc (sizeof(NODE *));
4. NODE *node1 = malloc (sizeof(NODE *));

Line 3 allocates a memory block the size of a pointer to a NODE structure. The cast allows you to assign it without any errors being thrown, and possibly without warnings. But it's not what you want.

Line 4 does the same as line 3. malloc returns a void * which doesn't require casting, but a good static analyzer should give you a warning.

Either way, lines 3 & 4 are recipes for buffer overflows and undefined behavior.

Lines 3 and four would be correct if written thus:

NODE **ptr = malloc(sizeof(NODE *));
kdopen
  • 8,032
  • 7
  • 44
  • 52
  • As a side note, a slightly cleaner way of implementing this sort of construct would be to use `sizeof(*node1)` instead. To someone unfamiliar with C, this may seem wrong, but `sizeof` is not a function, and nothing is actually being dereferenced. Using this notation, however, ensures that even if you later change the type of `node1`, your `sizeof` will still apply to whatever the type is. – eddiem Nov 18 '16 at 19:36
  • Agreed, and that's what I personally do. But I was trying to stay as close to the OP's original code as possible – kdopen Nov 18 '16 at 19:38
  • Casting the return value of `malloc` in C can _hide_ bugs (most prominently, forgetting `#include `) and is considered to be Wrong with a capital W. – zwol Nov 18 '16 at 19:39
  • They are *not* "just wrong". You simply can't access the allocated object through an expression of the type the pointer type is derived from, but the OP's code doesn't access the allocated object at all, so the code is fine (if useless). – EOF Nov 18 '16 at 19:39
  • Thanks for the answers. Can't vote you up at the moment. – James Real Nov 18 '16 at 19:41
3
// node1 is a pointer to (heap) memory block of size NODE
1. NODE *node1 = malloc (sizeof(NODE));    

// same as 1. (but 1. is the preferred way)
2. NODE *node1 = (struct node *) malloc (sizeof(NODE));

// (Wrong) node1 is a pointer to (heap) memory block of size pointer to NODE (a pointer to _usually_ 4 bytes)
//         but you cast it to pointer to NODE (a pointer to more than 4 bytes)
3. NODE *node1 = (struct node *) malloc (sizeof(NODE *));

// (Wrong) same as 3
4. NODE *node1 = malloc (sizeof(NODE *)); 

Stick with number 1. as it's the common way to allocate an object in the heap (dynamic allocation): Object *object = malloc(sizeof(Object)). You can forget about 2. 3. 4. which complicate the issue and really not what you need.

artm
  • 17,291
  • 6
  • 38
  • 54
-1

Given:

  1. NODE *node1 = malloc (sizeof(NODE));
  2. NODE *node1 = (struct node *) malloc (sizeof(NODE));
  3. NODE *node1 = (struct node *) malloc (sizeof(NODE *));
  4. NODE *node1 = malloc (sizeof(NODE *));

ALL ARE BAD

3 and 4 are really bad

3) and 4) are not allocating enough memory for the struct, but only enough memory to hold the pointer to a struct (say 4 bytes on a 32-bit system) and then telling the compiler to treat that memory as if it was enough to occupy a NODE

1) is correct way to allocate enough memory for the structure (caveat other issues described below)

2) is bad practice because what you really mean is:

  NODE *node1 = (NODE *) malloc(sizeof(NODE));

even though NODE is typedef struct node (2) says that you know that for sure.

Having said that,

All of them are bad practices for a reason explained later on below.

What you really want to do in practice is something like this:

Only use typedefs when you want to keep things really opaque about the inner structure (the way stdio does FILE) it's better practice to use struct foo bar; in your declaration than typedefing the struct. Especially when the structure is exposed to other parts of the program.

typedef is better used when you want to ensure that a type is uniformly interpreted on different architectures, e.g. the use of typedef unsigned long long uint64_t vs typedef unsigned uint64_t on 32-bit and 64-bit intel x86 cpus in stdint.h

A good example

struct node {
     struct node *next;
     size_t n_dlen;
     void * n_data;
};


int main() {
    struct node *node;

    node = malloc(sizeof(*node)); 
}

Notice that I used sizeof(*node) instead of sizeof(struct node), because if I decide later on that node should be some other type say struct new_node * then all i have to do is change the declaration and the sizeof(*node) will still be okay.

Ahmed Masud
  • 21,655
  • 3
  • 33
  • 58
  • 3
    Read the OP's typedef again. – EOF Nov 18 '16 at 19:48
  • @EOF I think i did just that. – Ahmed Masud Nov 18 '16 at 19:49
  • 2
    Contrary to your assertion, `NODE` is `typedef`d to `struct node` in the OP's code, rather than `struct node*` (which would indeed be a bad idea). – EOF Nov 18 '16 at 19:50
  • Looks like there are more than a dozen ways to do that. Damn, I wished I could be as good as you people. – James Real Nov 18 '16 at 19:52
  • @EOF you really should read my answer, nowhere did I say that NODE was a typedef to `struct node *1 ... actually where it did say it, it was a typo, and the context should have cleared that up – Ahmed Masud Nov 18 '16 at 20:53
  • 1
    @AhmedMasud You *do* realize that the edit history of your answer is visible to everyone? Claiming that the answer didn't say something that it definitely did is not a good idea. – EOF Nov 19 '16 at 07:41
  • @EOF err you do realize that I mentioned that in my comment right? – Ahmed Masud Nov 19 '16 at 22:47
  • @AhmedMasud: Your comment amounts to "I didn't do this thing, and if I *did*, you should have known I didn't mean it." This is so inane it's not worth entertaining. – EOF Nov 21 '16 at 21:43