1
struct node{
  int w;
  int x;
}
struct node *item;
  1. Please explain the difference between these statements.
  2. i read as typecasting after malloc is not needed. why is that so?

    1) item = malloc(sizeof(*item));

    2) item = malloc(sizeof(struct node));

    3) item = (struct node *) malloc(sizeof(struct node));

mpromonet
  • 11,326
  • 43
  • 62
  • 91
user1762571
  • 1,888
  • 7
  • 28
  • 47
  • 4
    1 and 2 are the same. don't use 3: http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc – Jason Hu Feb 26 '15 at 19:32
  • 3
    1) is slightly better IMHO because if for some reason the type of `item` changes to another type of pointer you'll have to change less code. – Joël Hecht Feb 26 '15 at 19:38
  • Not so! 1) and 3) are fine, 1) is more C-ish, 3) give portability to C++. Don't use 2) because it is error prone. – chqrlie Feb 26 '15 at 20:00
  • casting return of `malloc()` prevents 3 from being a good choice (this _is_ tagged C). ***1)*** is probably better of the first two, for the reasons given by @JoelHecht. – ryyker Feb 26 '15 at 20:31
  • 1
    Note that `sizeof` is an operator, not a function; you only need to surround the operand in parentheses if the operand is a type name. So you could write `sizeof *item` or `sizeof (struct node)`. I prefer the former for a number of reasons, not the least of which is that using the expression `*item` will protect you if someone decides to change the base type (say, from `struct node` to `struct another_node_with_a_different_size`). – John Bode Feb 26 '15 at 21:13
  • " i read as typecasting after malloc is not needed. why is that so?" - why would you think it is needed in the first place? If you try to explain why it is needed you will find yourself unable to come up with anything sensible – M.M Feb 26 '15 at 22:00

4 Answers4

2

All methods will give you exactly the same result in this particular case.

1:

item = malloc(sizeof(*item));

*item is a struct node, therefore sizeof(*item) will return the size of a struct node, and the line will allocate memory for a single node. This is sometimes to be preferred to 2), as if you were to refactor item to be a different type, you do not need to change this line.

2:

item = malloc(sizeof(struct node));

This line is fairly self-explanatory. As mentioned before, though, if item were to change type, you would need to change this line as the amount of memory allocated will no longer be correct (except, perhaps, by pure luck).

3:

malloc returns a pointer to void, or void *. This is automatically converted to the proper type even without a cast. In fact, several people (myself included) will advise you strongly not to use the typecast ever. For more details, read the accepted answer to this question

Community
  • 1
  • 1
wolfPack88
  • 4,163
  • 4
  • 32
  • 47
  • @ wolfPack88 How can *item be a data type . item is initially a pointer to nothing. Even if had pointed to something,then by dereferencing it we dont get a data type – Achyuta Aich Feb 26 '15 at 19:43
  • @jaig: Please don't leave an answer when you wish to comment. With that said, to answer your question: It doesn't matter if `item` points to anything or not, as `item` was declared as a `struct node *`, when `*item` is a `struct node`. For example, let's look at `int a = 4; int *b; b = &a;` In this example, `b` is a pointer to `int`, and `*b` evaluates to 4, i.e., `*b` is an `int`. `*b` was an `int` even before the line `b=&a`, but evaluating `*b` would have yielded a garbage result. – wolfPack88 Feb 26 '15 at 19:57
2

Let's look at these:

1) item = malloc(sizeof(*item));

The above code is the classic way in C. It may be safer to use calloc as it will clear to allocated structure to all bits zero. A useful side effect that will save you if you later add fields to the structure and forget to initialize the in the code that follows the malloc in every instance of an allocation. I would so recommend:

1 preferred) item = calloc(1, sizeof(*item)); 

The alternative below assumes item is a pointer to struct node. It may be true when you first write the code and may become false if you later change the type of item or if you copy to code to a different place where item is a different type and you forget to make the change.

2) item = malloc(sizeof(struct node));

The last option exhibits another issue:

3) item = (struct node *) malloc(sizeof(struct node));

The issue of casting the return value of malloc has been commented a lot elsewhere. It is necessary in C++ and considered useless and potentially error prone in C. There is a context in which it may actually serve a good purpose:

#define alloc_node()  ((struct node*)calloc(1, sizeof(node)))

item = alloc_node();

Here it is advisable to use the cast in order to detect allocations of the wrong type. If item is not a pointer to a struct node, the compiler will complain and prevent the bug.

In conclusion, prefer calloc over malloc, and prefer sizeof(*item) over sizeof(node node). If the context does not allow to use sizeof(*dst), as above or within an expression or a function argument, then use the cast.

chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • the macro one is really a very bad style. it hides a lot of essential things without providing any convenience. preferred way: `#define ALLOC_NODE(var) struct node *var = malloc(sizeof *var)`. – Jason Hu Feb 26 '15 at 20:22
  • `alloc_node()` is a poor man's inline function without side affects. Your proposal will fail on very simple cases: `ALLOC_NODE(*array)`. The naive user will have a hard time understanding what went wong ;-) – chqrlie Feb 26 '15 at 20:29
  • Also, I am not advocating the use of this macro, I am insisting on the use of the cast in this case. – chqrlie Feb 26 '15 at 20:30
  • that's not true. my style happens every where in linux kernel code. every interface has its usage and mine does too. in c, casting `malloc` is generally not recommended. there was a big discussion in my link in the comment. – Jason Hu Feb 26 '15 at 20:33
  • You are missing the point. If the context does not allow for syntax 1), using the cast will catch type mismatches that will go unnoticed with syntax 2). The discussion is very interesting, it shows there are good reasons for both syntaxes, depending on context. (see http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc ) – chqrlie Feb 26 '15 at 20:39
  • Consider variation on style 1) `item = malloc(sizeof *item);` - dropped unneeded `()`. – chux - Reinstate Monica Feb 26 '15 at 23:05
  • Indeed parentheses are only required for types and `sizeof` has the precedence of all prefix operators. This set of extra parentheses does not hurt and as, a matter of personal taste, looks more consistent to me. I never type spaces between monadic operators and their operand, and dont want to bend this stylistic rule for `sizeof`. – chqrlie Feb 26 '15 at 23:34
2

Functionally, all three are equivalent.

Stylistically, 1 is the preferred option (at least by me and a number of other C programmers). Note that sizeof is an operator, not a function, and parentheses are only required when the operand is a type name. So you could write sizeof *item as opposed to sizeof (*item) - the parens don't hurt anything except readability. Also note that sizeof does not attempt to evaluate its argument (it won't try to dereference item); all it does is compute the number of bytes required by the resulting type, and it does that at compile time, not run time.

As far as C is concerned, you should not cast the result of malloc. As of the 1989 standard it's no longer necessary; malloc returns a void *, which can be assigned to any other pointer type without needing an explicit cast1.

Under the C89 standard, casting the result of malloc could suppress a useful diagnostic if you forgot to include stdlib.h or otherwise didn't have a declaration for malloc in scope. C89 still allowed implicit int declarations; if the compiler saw a function call in the code without a declaration, it assumed the function returned an int value. If you left off the cast, the compiler would complain that you were trying to assign an integer value to a pointer object. With the cast, the compiler would compile the code without a diagnostic, but you'd be converting a pointer value to an int and back to a pointer again, which may or may not work.

The C99 standard got rid of implicit int declarations, so that isn't a problem anymore; cast or not, you'll get a diagnostic if you forget to include stdlib.h. You should still leave the cast off for readability and maintainability reasons; all it does is add useless visual clutter. If you change the declaration of the target (from struct foo * to struct bar *), then you'd have to update the cast as well, which is just extra work, and if you forget to do it then it's a bug. item = malloc( sizeof *item ) will always do the right thing without any further maintenance, no matter what changes you make to the type of item.

C++, being a different language from C with different rules, does not allow you to assign a void * to another pointer type without an explicit cast, so you would have to cast the result of malloc in that case. This is an issue if you're migrating from C to C++ and haven't redone all your dynamic memory management code (which is the only excuse for using malloc in C++ code). As a matter of course, you should not use malloc/calloc/realloc in C++; either use one of the standard containers, or use the new operator.


1 - Prior to the C89 standard, malloc, calloc, and realloc all returned char *, so a cast was required if the target was a different pointer type. Unfortunately, the habit persisted even though it was no longer necessary
John Bode
  • 119,563
  • 19
  • 122
  • 198
1

Form 2 is bad because you may accidentally allocate the wrong amount of memory.

Form 1 and Form 3 both allow the programmer to do a visual check that allocation is correct:

   item = malloc(sizeof(*item));
// ^^^^                 ^^^^^

   item = (struct node *) malloc(sizeof(struct node));
//        ^^^^^^^^^^^^^^^              ^^^^^^^^^^^^^

Once you learn that varname should go with *varname, or that (Typename *) should go with (Typename), then you are more likely to quickly detect size errors.

As explained on the main thread about this, in C89 Form 3 was dangerous whereas Form 1 was safe. In C99 and later that rationale is less compelling, however Form 1 has the benefits of being more compact, and requiring less maintenance. If the type of item is changed then any Form 3 lines will trigger a compilation error, whereas Form 1 lines will just automatically work correctly with the updated type.

Community
  • 1
  • 1
M.M
  • 138,810
  • 21
  • 208
  • 365