C struct and malloc problem (C)

Question

It's amazing how even the littlest program can cause so much trouble in C.

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

typedef struct node {
    int value;
    struct node *leftChild;
    struct node *rightChild;
} node;

typedef struct tree {
    int numNodes;
    struct node** nodes;
} tree;

tree *initTree() {
    tree* tree = (tree*) malloc(sizeof(tree));
    node *node = (node*) malloc(sizeof(node));
    tree->nodes[0] = node;
    return tree;
}

int main() {
    return 0;
}

The compiler says:

main.c: In function 'initTree':
main.c:17: error: expected expression before ')' token 
main.c:18: error: expected expression before ')' token

Can you please help?

The cause of the error (name clashes) are not because of C. You'd get an error in many languages. (But yes, the error message is incomprehensible.) — user395760
– user395760, Commented Jan 22, 2011 at 15:20

wkl · Accepted Answer · 2011-01-22 15:14:36Z

14

You're using two variables named tree and node, but you also have structs typedefed as tree and node.

Change your variable names:

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

typedef struct node {
    int value;
    struct node *leftChild;
    struct node *rightChild;
} node;

typedef struct tree {
    int numNodes;
    struct node** nodes;
} tree;

tree *initTree() {
   /* in C code (not C++), don't have to cast malloc's return pointer, it's implicitly converted from void* */
   tree* atree = malloc(sizeof(tree)); /* different names for variables */
   node* anode = malloc(sizeof(node));
   atree->nodes[0] = anode;
   return atree;
}

int main() {
    return 0;
}

answered Jan 22, 2011 at 15:14

wkl

80.4k16 gold badges171 silver badges178 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Gal Over a year ago

So should I never try to cast a void returned from malloc?

wkl Over a year ago

@sombe - you don't have to (in C), the conversion from void * to your pointer type is implicit.

Jens Gustedt Over a year ago

@sombe, yes. If you do, you might hide some implicit casts to int if the prototype of malloc is not included.

detunized · Accepted Answer · 2011-01-22 15:13:37Z

5

tree and node is your case are type names and should not be used as variable names later on.

tree *initTree() {
    tree *myTree = (tree*) malloc(sizeof(tree));
    node *myNode = (node*) malloc(sizeof(node));
    myTree->nodes[0] = myNode;
    return myTree;
}

answered Jan 22, 2011 at 15:13

detunized

15.3k3 gold badges50 silver badges66 bronze badges

Comments

user541686 · Accepted Answer · 2011-01-22 15:12:35Z

3

Change (tree*) and (node*) to (struct tree*) and (struct node*). You can't just say tree because that's also a variable.

answered Jan 22, 2011 at 15:12

user541686

213k133 gold badges563 silver badges935 bronze badges

7 Comments

user395760 Over a year ago

Not necessary (typedef struct tree {} tree).

user395760 Over a year ago

I tried it now, and it seems to compile. However, it works without struct (but only when the variable has a name not clashing with the type name).

user541686 Over a year ago

@delnan: Exactly my point... hence why he needs to say struct if he doesn't change the rest of his code.

user395760 Over a year ago

If he doesn't change the rest of the code, yes. But the majority (look at the votes) seems to think that renaming the variables is the real solution (it also allows dropping the arguably unnecessary struct before the typename).

user395760 Over a year ago

But if there is a better solution, why not propose the better solution (and perhaps mention inferior ones)?

|

Charlie Skilbeck · Accepted Answer · 2011-01-22 15:17:55Z

2

Change the body of initTree as follows:

tree* myTree = (tree *)malloc(sizeof(tree));
node *myNode = (node *)malloc(sizeof(node));
myTree->nodes[0] = myNode;
return myTree;

answered Jan 22, 2011 at 15:17

Charlie Skilbeck

1,1412 gold badges15 silver badges40 bronze badges

Comments

orlp · Accepted Answer · 2011-01-22 15:17:30Z

1

Don't use typedef'ed names as variable names, and there is not need to cast malloc(); in C.

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

typedef struct node {
    int value;
    struct node *leftChild;
    struct node *rightChild;
} node;

typedef struct tree {
    int numNodes;
    struct node** nodes;
} tree;

tree *initTree() {
    tree->nodes[0] = malloc(sizeof(node));
    return malloc(sizeof(tree));
}

int main() {
    return 0;
}

answered Jan 22, 2011 at 15:17

orlp

119k39 gold badges226 silver badges324 bronze badges

Comments

Infinite · Accepted Answer · 2011-01-22 15:53:10Z

0

I second that Mehrdad's explanation is to the point.

It's not uncommon that in C code you define a variable with the same name as the struct name for instance "node node;". Maybe it is not a good style; it is common in, e.g. linux kernel, code.

The real problem in the original code is that the compiler doesn't know how to interpret "tree" in "(tree*) malloc". According to the compiling error, it is obviously interpreted as a variable.

answered Jan 22, 2011 at 15:53

Infinite

3,4685 gold badges29 silver badges38 bronze badges

Comments

datenwolf · Accepted Answer · 2011-01-22 15:57:33Z

Apart from the original question, this code, even in it's correct forms will not work, simply due to the fact that tree::nodes (sorry for the C++ notation) as a pointer to a pointer will not point to anything usefull right after a tree as been malloced. So tree->nodes[0] which in the case of ordinary pointers is essentially the same like *(tree->nodes), can't be dereferenced. This is a very strange head for a tree anyway, but you should at least allocate a single node* to initialize that pointer to pointer:

tree *initTree() {
   /* in C code (not C++), don't have to cast malloc's return pointer, it's implicitly converted from void* */
   tree* atree = malloc(sizeof(struct tree)); /* different names for variables */

   /* ... */

   /* allocate space for numNodes node*, yet numNodes needs to be set to something beforehand */
   atree->nodes = malloc(sizeof(struct node*) * atree->numNodes);

   node* anode = malloc(sizeof(struct node));
   atree->nodes[0] = anode;
   return atree;
}

William Pursell · Accepted Answer · 2011-01-22 20:38:50Z

0

Interestingly, it does compile cleanly if you simply write the allocations as:

tree *tree = malloc( sizeof *tree );

It is often considered better style to use "sizeof variable" rather than "sizeof( type )", and in this case the stylistic convention resolves the syntax error. Personally, I think this example is a good case demonstrating why typecasts are generally a bad idea, as the code is much less obfuscated if written:

struct tree *tree = malloc( sizeof *tree );

answered Jan 22, 2011 at 20:38

William Pursell

214k49 gold badges279 silver badges317 bronze badges

Collectives™ on Stack Overflow

C struct and malloc problem (C)

8 Answers 8

3 Comments

Comments

7 Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

8 Answers 8

3 Comments

Comments

7 Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related