Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

What is the correct way to temporarily cast void* for arithmetic?

I am C novice but been a programmer for some years, so I am trying to learn C by following along Stanford's course from 2008 and doing Assignment 3 on Vectors in C.

It's just a generic array basically, so the data is held inside a struct as a void *. The compiler flag -Wpointer-arith is turned on so I can't do arithmetic (and I understand the reasons why).

The struct around the data must not know what type the data is, so that it is generic for the caller.

To simplify things I am trying out the following code:

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

typedef struct {
    void *data;
    int aindex;
    int elemSize;
} trial;

void init(trial *vector, int elemSize)
{
    vector->aindex = 0;
    vector->elemSize = elemSize;
    vector->data = malloc(10 * elemSize);
}

void add(trial *vector, const void *elemAddr)
{
    if (vector->aindex != 0)
        vector->data = (char *)vector->data + vector->elemSize;

    vector->aindex++;
    memcpy(vector->data, elemAddr, sizeof(int));

}

int main()
{
    trial vector;
    init(&vector, sizeof(int));

    for (int i = 0; i < 8; i++)
        {add(&vector, &i);}

    vector.data = (char *)vector.data - ( 5 * vector.elemSize);
    printf("%d\n", *(int *)vector.data);
    printf("%s\n", "done..");

    free(vector.data);
    return 0;
}

However I get an error at free with free(): invalid pointer. So I ran valgrind on it and received the following:

==21006==  Address 0x51f0048 is 8 bytes inside a block of size 40 alloc'd
==21006==    at 0x4C2CEDF: malloc (vg_replace_malloc.c:299)
==21006==    by 0x1087AA: init (pointer_arithm.c:13)
==21006==    by 0x108826: main (pointer_arithm.c:29)

At this point my guess is I am either not doing the char* correctly, or maybe using memcpy incorrectly

like image 411
Sam Hammamy Avatar asked May 23 '18 13:05

Sam Hammamy


2 Answers

This happens because you add eight elements to the vector, and then "roll back" the pointer by only five steps before attempting a free. You can easily fix that by using vector->aindex to decide by how much the index is to be unrolled.

The root cause of the problem, however, is that you modify vector->data. You should avoid modifying it in the first place, relying on a temporary pointer inside of your add function instead:

void add(trial *vector, const void *elemAddr, size_t sz) {
    char *base = vector->data;
    memcpy(base + vector->aindex*sz, elemAddr, sz);
    vector->aindex++;
}

Note the use of sz, you need to pass sizeof(int) to it.

Another problem in your code is when you print by casting vector.data to int*. This would probably work, but a better approach would be to write a similar read function to extract the data.

like image 151
Sergey Kalinichenko Avatar answered Sep 28 '22 05:09

Sergey Kalinichenko


If you don't know the array's data type beforehand, you must assume a certain amount of memory when you first initialize it, for example, 32 bytes or 100 bytes. Then if you run out of memory, you can expand using realloc and copying over your previous data to the new slot. The C++ vector IIRC follows either a x2 or x2.2 ratio to reallocate, not sure.

Next up is your free. There's a big thing you must know here. What if the user were to send you a memory allocated object of their own? For example a char* that they allocated previously? If you simply delete the data member of your vector, that won't be enough. You need to ask for a function pointer in case the data type is something that requires special attention as your input to add.

Lastly you are doing a big mistake at this line here:

if (vector->aindex != 0)
    vector->data = (char *)vector->data + vector->elemSize;

You are modifiyng your pointer address!!! Your initial address is lost here! You must never do this. Use a temporary char* to hold your initial data address and manipulate it instead.

like image 29
SenselessCoder Avatar answered Sep 28 '22 06:09

SenselessCoder