Search code examples
cmallocuint32-t

correct use of malloc in function with passed uint32_t array pointer


I'm having difficulty using malloc in a function where I read a binary file with 4 byte unsigned integers, free the passed array reference, remalloc it to the new size and then try to access members of the array. I think the problem is due to the uint32_t type as the array seems to be being seen as a array of 8 byte integers rather than 4 byte ones. Not exactly sure where I am going wrong, it might be with the use of malloc, IE maybe I need to instruct it to create uint32_t types in a different way to what I am doing, or maybe its something else. Code:

#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <sys/mman.h>
#include <unistd.h>
#include <string.h>
#include <stdint.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <malloc.h>

int filecheck (uint32_t **sched, int * count) {
    int v, size = 0;
    FILE *f = fopen("/home/pi/schedule/default", "rb");
    if (f == NULL) { 
        return 0; 
    } 
    fseek(f, 0, SEEK_END);
    size = ftell(f);
    fseek(f, 0, SEEK_SET);
    int schedsize = sizeof(uint32_t);
    int elementcount = size / schedsize;
    free(*sched);
    *sched = malloc(size);
    if (elementcount != fread(sched, schedsize, elementcount, f)) { 
        free(*sched);
        return 0;
    } 
    fclose(f);
// This works correctly and prints data as expected
    for (v=0;v<elementcount;v++) { 
        printf("Method 1 %02d %u \n", v, ((uint32_t*)sched)[v]);
    }

// This skips every other byte byt does not print any numbers > 32 bit unsigned
    for (v=0;v<elementcount;v++) { 
        printf("Method 2 %02d %u \n", v, sched[v]);
    }
// This treats the binary file as if it was 64 bit uints, printing 64 bit numbers
    for (v=0;v<elementcount;v++) { 
        printf("Method 3 %02d %lu \n", v, sched[v]);
    }
    *count = elementcount;
    return 1;
}

int main (){
    uint32_t *sched = NULL;
    int i, count = 0;
    if (filecheck(&sched, &count)) {
        for (i=0;i<count;i++) { // At the next line I get a segmentation fault
            printf("Method 4 %02d %u\n", i, sched[i]);
        }
    } else {
        printf("Error\n");
    }
    return 0;
}

I added the file reading code as requested. Trying to access the array sched in main() the way I am doing will print the data I am reading as if it was 8 byte integers. So I guess sched is being seen as an array of 64 bit integers rather than 32 bit as I have defined it as. I guess this is because I freed it and used malloc on it again. But I believe I instructed malloc that the type it should create is a uint32_t so I am confused as to why the data is not being treated as such.

Edit: Found a way to make it work, but not sure if its right (method 1). Is this the only and cleanest way to get this array treated as uint32_t type? Surely the compiler should know what type I am dealing with without me having to cast it every time I use it.

Edit: Actually when I try to access it in main, I get a seg fault. I added a comment in the code to reflect that. I didnt notice this before as I was using the for print loop in several places to trace how the data was being viewed by the compiler.

Edit: Not sure if there is a way I can add the binary data file. I am making it by hand in a hex editor and its exact content is not that important. In the bin file, FF FF FF FF FF FF FF FF should be read as 2 uint32_t types, rather than as 1 64 bit integer. Fread AFAIK does not care about this, it just fills the buffer, but stand to be corrected if thats wrong.

TIA, Pete


Solution

  • // This works correctly and prints data as expected
        for (v=0;v<elementcount;v++) { 
            printf("Method 1 %02d %u \n", v, ((uint32_t*)sched)[v]);
        }
    

    This is not quite right, because sched is a uint32_t**, and you allocated to *sched. And also because %u is not how you should print uint32_t. First dereference sched and then apply the array index access.

    You want printf("%02d %"PRIu32"\n", v, (*sched)[v]));

    // This skips every other byte byt does not print any numbers > 32 bit unsigned
        for (v=0;v<elementcount;v++) { 
            printf("Method 2 %02d %u \n", v, sched[v]);
        }
    

    Yeah, that's because sched[v] is a uint32_t*, and since it's a pointer type and you're probably running on a 64-bit machine, it's probably 64 bits... so you're iterating with 8-byte increments instead of 4-byte ones and trying to print pointers as %u. It also goes out of bounds because 8*10 is larger than 4*10, and that is likely to cause a segmentation fault.

    // This treats the binary file as if it was 64 bit uints, printing 64 bit numbers
        for (v=0;v<elementcount;v++) { 
            printf("Method 2 %02d %lu \n", v, sched[v]);
        }
    

    This is like the second example only you're printing with the marginally more appropriate but still incorrect %lu.

    You need to fread() into *sched instead of sched, as well, or you will cause UB and this will very likely cause a segmentation fault when you try to read the array.

    Also, your for loop in main should never run since you aren't setting count to anything in filecheck so it should still be zero (in the absence of UB, at least).

    Other comments:

    • As you've just learned, don't cast the result of malloc() in C.
    • main() should return a value. 0 if everything went fine.
    • You can write some uint32_t to a file as bytes using fwrite(), of course.
    • Check the result of malloc() to see if it succeeded.
    • Use size_t for size_t values instead of int, and print accordingly (%zu).