C memcpy causing segmentation fault

1.7k Views Asked by At

So I'm trying to implement a simple memory pool as part of an assignment for University however I've ran in to trouble with storing values inside the memory I have allocated.

This is my main.c file:

#include <stdio.h>
#include "Pool.h"

int main(int argc, char** argv)
{
    Pool* pool = allocate_pool(64);

    printf("Pool Size: %d bytes...\n", pool->size_bytes);

    int* a = (int*)100;

    store_in_pool(pool, 20, sizeof(int), a);

    void* ap = retrieve_from_pool(pool, 20, sizeof(int));

    printf("%d\n", ap);

    free_pool(pool);

    return 0;
}

My Pool.h file:

#ifndef ASSIGNMENT_2_POOL_H
#define ASSIGNMENT_2_POOL_H

typedef struct MemoryPool
{
    int size_bytes;
    void* data;
} Pool;

Pool* allocate_pool(int size_bytes);
void  free_pool(Pool* pool);
void  store_in_pool(Pool* pool, int offset_bytes, int size_bytes, void* object);
void* retrieve_from_pool(Pool* pool, int offset_bytes, int size_bytes);

#endif

And my Pool.c file:

#include <stdlib.h>
#include <string.h>
#include <stdio.h>
#include "Pool.h"

Pool* allocate_pool(int size_bytes)
{
    Pool* pool = (Pool*)malloc(sizeof(Pool*));
    pool->size_bytes = size_bytes;
    pool->data = malloc(size_bytes);

    int i = 0;
    while(i < pool->size_bytes)
    {
        void* temp = (int*)pool->data + i++;
        temp = 0;
    }

    return pool;
}

void free_pool(Pool* pool)
{
    free(pool->data);
    free(pool);
}

void store_in_pool(Pool* pool, int offset_bytes, int size_bytes, void* object)
{
    memcpy((void*)((char*)pool->data + offset_bytes), object, size_bytes);
}

void* retrieve_from_pool(Pool* pool, int offset_bytes, int size_bytes)
{
    return (void*)((char*)pool->data + offset_bytes);
}

The issue appears whenever I call 'store_in_pool' which contains a line that calls memcpy. I'm not sure what the issue is as I'm certain I'm passing the correct values to the function however a segmentation fault occurs every time I try and run the program.

What could be the cause of the problem?

3

There are 3 best solutions below

8
On BEST ANSWER

The problem is here:

Pool* pool = (Pool*)malloc(sizeof(Pool*));

On 32 bit systems sizeof(Pool*)==4. This is because the Pool* argument indicates that you want the size of a pointer to Pool. Pointer sizes are constant (4 on 32 bit, 8 on 64 bit). It should be:

Pool* pool = (Pool*)malloc(sizeof(Pool));

In this case the size of the the Pool struct will be sent to malloc. One other thing I noticed in your code. It's not a bug, per se, but it is code with zero effect:

while(i < pool->size_bytes)
{
    void* temp = (int*)pool->data + i++;
    temp = 0;
}

You are setting the temp pointer to NULL, essentially, not setting the variable to which it pointed to 0. This means that your pool->data is never initialized. One way to modify it:

while(i < pool->size_bytes)
{
    char* temp = (char*)pool->data + i++;
    *temp = 0;
}

Or simply:

memset(pool->data, 0, pool->size_bytes);

Or just catch it at the source and remove the initialization code if all you need is initialization to 0:

   pool->data = calloc(1, pool->size_bytes);

In this case, calloc sets all bytes to 0.

0
On

The segmentation fault is happening because of this:

int* a = (int*)100;

This sets a to be a pointer to the address 100, which is not part of your accessible memory. So when you then try to copy from that address with memcpy(), you get a fault.

If you want a to point to an integer with the value 100, the correct way is:

int aval = 100;
int *a = &aval;

You also need to fix the way you call malloc() in allocate_pool:

Pool* pool = malloc(sizeof Pool);

Your code is just allocating enough space for a pointer, not the entire Pool structure.

The while loop that looks like it's trying to initialize data to zero is also wrong. You can simply use memset:

memset(pool->data, 0, size_bytes);

You could also have used calloc() instead of malloc() to allocate the space, since it automatically initializes the space to zeroes.

2
On

Change this:

Pool* pool = (Pool*)malloc(sizeof(Pool*));

to this:

Pool* pool = malloc(sizeof Pool);