Memory leak on reading binary file

[summer linden]

here the raw data spec

and there is the .h of the BN_Halftoning object:

class BN_Halftoning
{
    private:
        colorGrid3d     _blueNoise;        /* [z(depth)][y(width)][x(height)] */
        int_fast32_t    _tileHeight;
        int_fast32_t    _tileWidth;
        int_fast32_t    _tileDepth;

        void loadBlueNoiseFromFile(std::string fileName); /* takes .raw files */

    public:
        BN_Halftoning();
        virtual ~BN_Halftoning();
};

[glacial slate]

problem is RAII, if you throw exception, malloc() will leak

you need to wrap everything in C++ try/catch

also why are you converting void* to int_fast32_t* to char*?

also I would use int_fast32_t not auto
and hope the compiler does the right thing

also if you are doing weird pointer arithmetic, why are you doing reinterpret_cast<char*>

also shouldn't you malloc sizeof() something known?

[summer linden]

I only use int_fast32_t to make sure it has the capacity to read the 32bits values of the file

I use auto in for loops to follow the style of my team

[glacial slate]

let, var, auto, should burn in hell 🔥

auto was used because STL iterator<> syntax is insane

[summer linden]

nah

auto was used because at some point in the past it was the default to a few use cases

[glacial slate]

in the past?

it was a useless keyword, since all variable who are not register or static are auto

[summer linden]

then c went back to strongly typed - no exception allowed

but anyway auto in for loops doesn't cause any problems ever

and that's not the main topic

[glacial slate]

auto z = 0; could be int or could be int_fast32_t

because 0 decides what is auto type

I think

in your case you might be comparing int with int_fast32_t

[summer linden]

which is essentialy the same, int_fast32_t is just enforcing it

but int is as "undefined" as auto

[glacial slate]

you shouldn't mix type when looping and checking

[summer linden]

here, the range of value x, y, or z can take is relatively small regardless

so it will either pic a value that makes the comparison with the tileHeight easy

[glacial slate]

also when you read binary int

[summer linden]

or just int32

[glacial slate]

it can be little endian or big endian

on most processor it's little endian

but not powerpc and some arm and other RISC

[summer linden]

here it's little andian, I checked

[glacial slate]

anyway malloc is a problem, why are you not using new

[summer linden]

but isn't the maloc called after the throw btw?

[glacial slate]

in a unique_ptr<char*>

the rest of the code might throw

then malloc is not RAII

therefore free() is never call

therefore it leaks on exception

[summer linden]

the rest of the code won't, it would just crash

[glacial slate]

how do you know?

[summer linden]

and I won't check if the file is properly formated: it is

[summer linden]

because .read doesn't throw

[glacial slate]

easiest is just add try/catch

then you can 100% be sure

[summer linden]

The error was caused by my size

basically .read takes the byte-count as buffer size, not the bit-cout

so it was pumping a 256bits long value as the matrix size

then the vector would be a random number of 256bits (a VERY big number), cubed

so it tried to create a vector long of 62000*43546*862211 elements ._.

[glacial slate]

and it throw OOM or buffer overflow or something

and crashed

makes sense

[summer linden]

it didn't crash actually