-2

I cannot see anything wrong with this.

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

/* Reference:
    (0 offset) uint32_t size;
    (+4 offset) uint32_t elementSize;
    (+8 offset) uint32_t lastIndex;
    (+12 offset) void* data
*/

void* create_pinned_array(uint32_t elementSize, uint32_t numberOfElements, size_t sizeOfData, void* data)
{
    uint32_t size = sizeOfData + 12;
    void* memory = malloc(size);
    memset(memory, size, 4); // size
    memset(memory + 4, elementSize, 4); // elementSize
    memset(memory + 8, numberOfElements-1, 4); // lastIndex
    memcpy(memory + 12, data, sizeOfData); // actual data

    return memory;
}

int main(void)
{
    int randomList[5] = { 1, 6, 2, 9, 8 };
    void* myIntegers = create_pinned_array(sizeof(int), 5, sizeof(randomList), randomList);

    FILE* testfp = fopen("testfile.bin", "wb");
    fwrite(myIntegers, sizeof(myIntegers), 1, testfp);
    fclose(testfp);

    free(myIntegers);
    return 0;
}

Each time I check with xxd, instead of an ordered binary file I am getting junk data which varies every time I execute the program.

RF1991
  • 2,037
  • 4
  • 8
  • 17
Hexa
  • 27
  • 4
  • 5
    `sizeof(myIntegers)` is the size of the pointer, not the mem block. – 001 Jul 19 '23 at 20:41
  • This makes wild assumptions about the endian form of those values that I hope are justified. – tadman Jul 19 '23 at 20:43
  • 1
    `memset(memory, size, 4);`?!?!?! Do you understand that `memset()` sets every byte of what is being set to the *same* byte value? – Andrew Henle Jul 19 '23 at 20:49
  • And please read both [**Flexible array member in C-structure**](https://stackoverflow.com/questions/3047530/flexible-array-member-in-c-structure) and [**How to correctly use flexible array member?**](https://stackoverflow.com/questions/68769314/what-is-a-flexible-array-member-in-a-struct) – Andrew Henle Jul 19 '23 at 20:53
  • @AndrewHenle i can't believe i made that mistake. Using memcpy instead fixed it. Thx – Hexa Jul 19 '23 at 21:03
  • @Hexa Simple assignment using code such as `memory->size = size;` is much preferred. Using `memcpy()` to an offset will be wrong if the type of `memory->size` changes (for example, to `size_t` on a 64-bit system), or if the layout of the structure changes. – Andrew Henle Jul 19 '23 at 21:09
  • @AndrewHenle yes but i am trying to build a data-type agnostic container kind of like a vector in c++ so using a struct api wouldn't work. – Hexa Jul 19 '23 at 21:46

2 Answers2

3

You are using memset when you mean to use memcpy.

memset(ptr, value, N)

Fills the bytes from ptr until ptr+n with value.

memcpy(ptr, src, N) copies N bytes from src into ptr.

Instead of this:

memset(memory, size, 4); // size
memset(memory + 4, elementSize, 4); // elementSize
memset(memory + 8, numberOfElements-1, 4); // lastIndex
memcpy(memory + 12, data, sizeOfData); // actual data

This:

uitn32_t lastIndex = numberOfElements-1;

memcpy(memory, &size, 4); // size
memcpy(memory + 4, &elementSize, 4); 
memcpy(memory + 8, &lastIndex, 4);   
memcpy(memory + 12, data, sizeOfData);

Or you could try this, which should be ok since the malloc'd memory would be aligned. (Some non-intel platforms, alignment is important if the pointer isn't on a 32-bit boundary)

uint32_t* header = (uint32_t*)memory;
header[0] = size;
header[1] = elementSize;
header[2] = nmberOfElements-1;
memcpy(memory + 12, data, sizeOfData);

This might not be your only bug.

selbie
  • 100,020
  • 15
  • 103
  • 173
2

Looks like you;'ve made a mistake with sizeof.

sizeof(myIntegers)

Returns the size of the void pointer, while in this case it appears create_pinned_array has returned a block of memory 32 bytes in size (assuming int is 4 bytes). Assuming a 64-bit machine, this means you're writing the first 8 bytes of your data.

Your create_pinned_array function should also check the return of malloc for success before proceeding to work with the results.

Chris
  • 26,361
  • 5
  • 21
  • 42