correct use of malloc in function with passed uint32_t array pointer

Question

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

You ask about data you read, without showing how you read it, and why it displays the way it does, without showing what that output was. Besides the cast malloc (which you shouldn't do in C), Your allocation calls appear correct for storing 10x uint32_t. The only thing off is the printf specifier, which should be formed via concatenation from the appropriate constant in inttypes.h when using uint32_t But how you're loading you're array and what you're loading it with is all on you. — WhozCraig
– WhozCraig, Commented May 8, 2015 at 3:55
To print a uint32_t correctly, do something like this: printf("%"PRIu32"\n", x); (that requires inttypes.h, of course). — Iskar Jarak
– Iskar Jarak, Commented May 8, 2015 at 3:58
Sorry - I will get to work shaving down the file reading bits and that will fill in the gaps I suppose. But to cut to the chase, its just that the data is not being printed correctly, its being indexed incorrectly too. I've played with lots of printf specifiers, and although some create different results than others, they all either skip every other byte, or view the data as 64 bit bytes. The indexing of the array is wrong. The pointer is being treated as a pointer to a 64 bit type, not a 32 bit type. — Pete
– Pete, Commented May 8, 2015 at 4:02
@WhozCraig thanks re the cast malloc, have read that link and will adjust accordingly. — Pete
– Pete, Commented May 8, 2015 at 4:31

Iskar Jarak · Accepted Answer · 2015-05-08 05:00:38Z

1

// 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).

edited May 8, 2015 at 5:00

answered May 8, 2015 at 4:36

Iskar Jarak

5,3757 gold badges41 silver badges61 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Pete Over a year ago

Ah! Was playing round with some of your suggestions but it still wasn’t working but then fread into *sched as you said and then it came together. After working through some other posts and tutorials I think I understand properly too. Could you expand on the last comment you made, re using size_t? Also, is including inttypes.h the only way to printf these uint32_t's? Thanks, Pete

Iskar Jarak Over a year ago

@Pete Yes, as far as I know, including inttypes.h is the only portable way to print uint32_t and co. Things like fread() take and return size_t, which is a special unsigned integer type guaranteed to be able to hold any legal size. While in this case, it's no big deal, if you were dealing with really big arrays this could catch you out. Implicit (automatic) casting size_t to/from int could also catch you out if you don't realise that it's happening. In the same vein, ftell returns a long - another potential gotcha if you're storing the result as int.

Iskar Jarak Over a year ago

@Pete Basically, just be aware of what types are expected or returned by the stdlib/system calls you're using and one day you might avoid a horrible bug. It matters particularly for writing portable code that will run on platforms with different type sizes. And if you think my answer helped, the way SO works is to consider accepting it

Pete Over a year ago

Absolutely Iskar, accepted right now, thanks very much too. Also for the extra info re the sizes, regarding ftell and such - I didn't really know about this and it points me towards the reading I need to do to make sure this simple little program is bug free which it needs to be as its long running on low powered hardware. Great answer, got me motoring again, plus extra info. Will plus one if I can too.

Collectives™ on Stack Overflow

correct use of malloc in function with passed uint32_t array pointer

1 Answer 1

4 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

4 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related