How to copy data to a pointer returned by malloc?

Question

Here are several snippets taken from real stackoverflow questions:

table *temp = malloc(sizeof(table));
temp = cursor->next;

char students[][50] = { "Alan", "Bob", "Charles", "James", "Peter" };
char (*heapStudents)[50] = malloc(sizeof(students));
heapStudents = students;

t = (struct carinfo_t *)malloc(sizeof(struct carinfo_t));
t = carbase;

char *str = (char *)malloc(20 * sizeof(char));
str = "This is a string";

struct node_t *temps = malloc(sizeof(struct node_t *));
temps = src;

As you can see, there is a clear pattern.

   p = malloc(sizeof(something));
   p = something_else;

Is there something wrong with it? Is it safe? Some people comment that "this is not how pointers work in C", but what do they actually mean by that?

This is a partial duplicate of [this question](https://stackoverflow.com/questions/62860912/c-alocate-memory-using-malloc/62861399#62861399) but I wanted to give a more general overview of the problem. — n. m. could be an AI, May 21 '22 at 13:46
There's not "something wrong with it". It is completely wrong. — William Pursell, May 21 '22 at 13:49
The title has no relationship to the question, and this question will not be discoverable by anybody needing this information in the future, nor would people who need it be searching for it in the first place. — Eric Postpischil, May 21 '22 at 15:05
@EricPostpischil Please feel free to propose a different title, or just edit as you see fit. I don't like it either but can't think of something more appropriate. — n. m. could be an AI, May 21 '22 at 15:35
@EricPostpischil I think I've found a good title. It makes little sense but that's intentional. — n. m. could be an AI, May 21 '22 at 17:03

n. m. could be an AI · Accepted Answer · 2022-05-21T17:12:23.320

Is there something wrong with it?

Yes, there is. In each of these cases there is a memory leak. In addition, some of these snippets may do the wrong thing (i.e. the program gives a wrong answer in addition to leaking memory, or just crashes).

In order to understand the memory leak, let's review what assignment does in C. In the words of the standard:

In simple assignment (=), the value of the right operand [...] replaces the value stored in the object designated by the left operand.

In simple down-to-earth words, the left operand forgets the value it was holding before, and starts holding something else.

Now let's recall that a pointer is a value that points to some data (an object, or a block of memory).

What are these values that are copied and forgotten in these statements?

p = malloc(sizeof(something));

// p stores the value returned by malloc, which is a pointer to 
// a block of memory. note that this value is the only pointer in 
// existence that points to that block of memory.

p = something_else;

// p forgets that it was storing a pointer to a block of memory 
// returned by malloc, and now stores a different pointer,
// presumably pointing to another block of memory. 
// the old pointer is now lost forever. there is no other pointer
// that points to the same block.

This is the very definition of memory leak.

Note that the block of memory itself is not touched or modified in any way, shape, or form. Only pointers are changed.

It's the confusion between the pointer and the pointed-to data that leads to this error. It seems that people commit it operate under a false impression that p = malloc(...) allocates space for data, and p = something_else assigns the data to space just allocated. This is indeed not how pointers work. Both statements only assign pointers, and do not touch pointed-to data.

What would be the right way to do this?

It depends on what is "this", i.e. on what you need. There is no universal recipe. In some cases you just want to copy a pointer. Drop the malloc line, and use the second assignment directly:

p = something_else; // no malloc here, just pointer assignment

In other cases you need to copy data pointed to by something_else into a new block of memory returned by malloc and now pointed by p (as opposed to something_else itself, which is a pointer):

p = malloc(sizeof(*p));           // note how sizeof is used here.
copy_data (p, something_else);    // always prefer this over sizeof(some_type)

Of course there is no such thing as copy_data in C. This is something you need to provide yourself, or use one of the standard functions. For example, if you need to copy a string:

p = malloc(strlen(something_else) + 1); // note no sizeof here. the size is dynamic
strcpy(p, something_else);              // also note +1 for the null terminator

// these two lines can be replaced by p = strdup(something_else);
// strdup is a function that has recently entered the C standard, but it was
// supported by all major compilers since the dawn of time

If you need to shallow-copy a struct:

p = malloc(sizeof(*p));
memcpy(p, something_else, sizeof(*p));
// or, if both p and something_else are declared 
// as pointers to struct something
*p = *something_else;

// if the struct contains pointers, this is probably not what you want

You could add that `memcpy(p, something_else, sizeof(*p))` is equivalent to `*p = something_else` if both `p` and `something_else` are defined as `struct something *`. — chqrlie, May 21 '22 at 15:14

How to copy data to a pointer returned by malloc?

1 Answers1

Linked

Related