0

I'm sorry for too specific question, but pointer stunts are very difficult to me, with my Py background.

Edited

I want to declare an array of structures:

struct key {         
  int* data;         
  struct node* left; 
  struct node* right;
};                   

//There is a problem too. Is it possible to return an array of structs from a function.                 
struct key *NewNode(int value) { 
  struct key** new_node;                          
  struct key* new_key;                            

  new_node = (struct key*)malloc(sizeof(new_key)); // warning is here
  new_key  = malloc(sizeof *new_key);             

  new_key->data = value;                          
  new_key->left = NULL;                           
  new_key->right = NULL;                          

  new_node[0] = new_key;                          

  return new_node;                      
}                 

// Somewhere in further code: 
realloc(new_node, sizeof *key);
new_node[1] = new_key; // Next key passed to the node.                               

How to declare array of structs without this warning?

assignment from incompatible pointer type

And please, describe to me, why is it happened? Because I still believe that the type of array members is struct key.

10
  • new_node is a pointer to pointer, hence the warning. Can you not just return new_key? Commented Apr 1, 2014 at 20:52
  • You are assigning a struct key * to a struct key **. If you want to do this you need to dereference your double pointer: *new_node = (struct key*)malloc(sizeof(new_key)); You also need to initialize your double pointer. Commented Apr 1, 2014 at 20:53
  • 1
    And don't cast the result of malloc either! Commented Apr 1, 2014 at 20:54
  • 1
    @HunterMcMillen Does that work, given that you don't know value that *new_node points to? Commented Apr 1, 2014 at 20:54
  • @AntonH I didn't notice it wasn't initialized. So no, in this case it doesn't because the double pointer isn't initialized, but presumably it would work if it was. Commented Apr 1, 2014 at 20:56

2 Answers 2

2

There are several problems here.

First of all, I'm assuming that the purpose of this code is to allocate and initialize a single object of type struct node; as such, new_node is superfluous; it serves no purpose. Just get rid of it completely and return new_key directly.

Secondly,

new_key  = malloc(sizeof(new_key));

won't allocate enough memory to store an object of type struct node; it only allocates enough space to store a pointer to struct node, because the type of the expression new_key is struct node *. You'll want to change that line to

new_key = malloc( sizeof *new_key );

Note that sizeof is an operator, not a function; parens are only necessary if the operand is a type name like int or float or struct whatchamacallit.

Finally, the line

new_key->data = value;

is a red flag; depending on how you call NewNode, this pointer may wind up being the same for all of your nodes. If your caller looks something like:

while ( not_done_yet )
{
  x = get_input();
  node = NewNode( &x );
  root = insert( root, node );
}

all of the nodes in your tree will wind up pointing to the same thing (x). Worse yet, if x goes out of scope, then suddenly all those pointers become invalid.

If your node is meant to store a single integer value, then it's better to make both the parameter and the data member regular ints, rather than pointers to int.

Summarizing:

struct node {         
  int data;         
  struct node* left; 
  struct node* right;
};                   

struct node *NewNode(int value) 
{
  struct node* new_key = malloc(sizeof *new_key);
  if ( new_key )
  {
    new_key->data = value;
    new_key->left = NULL;
    new_key->right = NULL;
  }
  return new_key;  
}                   

EDIT

If you want to create an array of nodes, do so in a function that calls NewNode, not in the NewNode function itself:

#define INITIAL_SIZE 1

struct node *CreateNodeArray( size_t *finalArraySize )
{
  *finalArraySize = 0;
  size_t i = 0;

  /**
   * For the purpose of this example, we'll start with an array of size
   * 1 and double it each time we hit the limit, but in practice you'd
   * want to start with a minimum size large enough to handle most
   * cases.
   */
  struct node *new_node = malloc( sizeof *new_node * INITIAL_SIZE );
  if ( new_node )
  {
    *finalArraySize = INITIAL_SIZE;

    while ( there_is_more_data() )
    {
      /**
       * First, check to see if we've hit the end of our array
       */
      if ( i == *finalArraySize )
      {
        /**
         * We've hit the end of the array, so we need to extend
         * it; in this case, we'll double its size
         */
        struct node *tmp = realloc( new_node, 
                                    sizeof *new_node * (*finalArraySize * 2) );              
        if ( tmp )
        {
          *finalArraySize *= 2;
          new_node = tmp;
        }
      }

      /**
       * Add a new node to the array
       */
      int data = getInput();
      new_node[i++] = NewNode( data ); 
    }
  }
  return new_node;
}

This code makes some assumptions about how you're building the individual nodes, but it should illustrate the point I'm trying to make, which is that you want cleanly separate building the array of nodes from building a single node.

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

3 Comments

+1 (but I still want to know where struct node* comes into this).
@WhozCraig: Oh for crap's sake, that snuck right by me. I plead lack of sleep.
@JohnBode, your answer makes my understanding much more clearer. But before I'll vote your answer up... I need to a new_node be an array of structs key. New node got only one member, but later amount of keys could be increased. I returned pointer to the 0 elem, because I don't know how to return an array of structs from a function. I edited my question.
1

The warning comes from your bogus cast. Remove it, you should not cast the value returned by malloc anyway.

Also, you malloc the wrong number of bytes to new_key on the line after.

You could have avoided both of these errors by using this idiom:

new_node = malloc( sizeof *new_node );
new_key = malloc( sizeof *new_key );

Your function also has a memory leak. You return new_node[0], leaking the memory new_node. I'm not exactly sure what you're trying to do (I'd expect that a function called "new node" would return a new struct node); but if you remove new_node entirely from the function then it still works just the same and doesn't leak memory.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.