lvandeve / lodepng

PNG encoder and decoder in C and C++.
zlib License
2.06k stars 421 forks source link

Null is passed in to free() #89

Open kyx0r opened 5 years ago

kyx0r commented 5 years ago

Hi there! This is a great lib and all, however when i was modifying its memory management code i noticed that function static void lodepng_free(void* ptr) gets a null passed in a lot of times.

The best way to reproduce is to run example_4bit_palette.cpp example and put something like if(!ptr) { printf("null ptr \n"); } and see stuff fall into console. It does not cause crash or any consequences it looks like, but still I would like to point this out, because if this is unintentional its good to know how the code behaves. And maybe fix this.

version string = 20190210

stinos commented 5 years ago

It's NULL not Null or null, and the both C anc C++ standards allow passing NULL to free in which case it does nothing, so I don't think this is an issue?

kyx0r commented 5 years ago

Yes, whatever you call it, NULL or nullptr or 0 well anything that describes EAX = 00000000.

As i said its not an issue as it does not crash, however when I replaced it with my custom memory zone allocator I had unexpected a crashes because of it. So that is the reason i reported this. To fix this so that no one else like me runs into this, just put a null check with return in that function, should be good.

stinos commented 5 years ago

Well, unless this isn't intentional, I think this should be rather be considered a doocumentation issue and/or an issue with custom code. I mean, lodepng uses C-style allocation. So if somebody implements custom allocation, he/she must follow that (pretty much canonical) allocation scheme. But you didn't figure out or forgot this means deallocation handles NULL. So IMO the fix is to document that explicitly, not add extra code which is going to be duplicate anyway in 99% of cases out there.