0

I'm working on a "Node" structure for a C project, and even though the question might be pretty trivial, I just can't figure out how to solve a pointer that accesses an invalid value.

Here is the structure code:

struct Node {
  // the data is stored as a void pointer
  void *data;
  // a pointer to the next node in the chain
  struct Node *next;
  // a pointer to the previous node in the chain
  struct Node *prev;
};

I'm using a constructor to initialize a new node, this is the code:

struct Node node_constructor(void *data, unsigned long size) {
  // create a Node instance to be returned
  struct Node node;

  // allocate space for the data if it is of a supported type
  node.data = malloc(size);
  memcpy(node.data, data, size);

  // initialize the pointers
  node.next = NULL;
  node.prev = NULL;

  return node;
}

Now, I want this node to store any type of data, so I thought I'd use a void pointer and a size attribute to store the data type in memory. Everything works fine with primitive data types (int, char, long, short, arrays of these types, etc.), but I also want this node to be able to store data types customized structures such as.

Here is an example:

// I want this "Test" struct to be stored in a node
struct Test {
  int value;
  char *key;
} test = {200, "test"};

void main() {
  // allocate space
  struct Node *node = (struct Node *) malloc(sizeof(struct Node));

  *node = node_constructor(&test, sizeof(test));
  struct Test *new_test = (struct Test *)&node->data;
  
// This will print a random value, as the pointer points to an uninitialized place in memory
  printf("%d\n", *(int *)&new_test->value);

  // This will print 200 as expected
  printf("%d\n", *(int *)&test.value);
}

Is it the way I'm initializing the node data, or the way I want to retrieve it? There are no errors or warnings, is just printing random values.

Even the smallest help is welcome and would help me tremendously. Thank you!! :D

GrahamTheDev
  • 22,724
  • 2
  • 32
  • 64
  • By using `memcpy`, you're assuming that the data is consecutive in memory. However, `char *key` in your structure points to a different place in memory, meaning that the data you're looking to copy is **not** consecutive in memory. –  Jan 30 '22 at 10:54
  • You may be able to fix it, by using `char key[5]` instead. –  Jan 30 '22 at 10:59
  • Hmm, not really, no, because I'm storing an entire structure, and I'm also trying to retrieve it as an entire structure, so this is really not the case. As @Eric Postpischil said, I was retrieving the address of the structure by using the ampersand (`&`), but I was expecting the values stored in those memory addresses :)) – Daniel Tanase Jan 30 '22 at 16:20

1 Answers1

3

struct Test *new_test = (struct Test *)&node->data; sets new_test to the address of node->data, not its value. You want new_test to point to where node->data points, not to node->data.

Change it to struct Test *new_test = node->data;.

Note that in addition to removing the &, I have removed the cast. It was hiding the error. Without the cast, the compiler prints an error message for struct Test *new_test = &node->data;, because the type of &node->data is not suitable for new_test. For example, Clang prints “incompatible pointer types initializing 'struct Test *' with an expression of type 'void **'”. With the correct code, the compiler does not complain because the type of node->data is void *, which may be implicitly converted to a pointer to any unqualified object type.

For the same reason, it is recommended not to cast the result of malloc: Casts in C may hide errors and should be avoided. For example, change struct Node *node = (struct Node *) malloc(sizeof(struct Node)); to struct Node *node = malloc(sizeof *node);.

Eric Postpischil
  • 195,579
  • 13
  • 168
  • 312