malloc returns not checked - should I worry?

[SoftwareDave]

I'm intending to supply a modified version of VLC to a customer. I'm currently at the stage where I've downloaded the 0.7.2 source and managed to compile it for Windoze under Cygwin, and I'm starting to examine the code to see where I need to make changes.
To my horror I find that many calls to malloc() in the source are unchecked for a null return. Example from libvlc.c:

Code: Select all

/* * Initialize hotkey handling */ var_Create( p_vlc, "key-pressed", VLC_VAR_INTEGER ); p_vlc->p_hotkeys = malloc( sizeof(p_hotkeys) ); /* Do a copy (we don't need to modify the strings) */ memcpy( p_vlc->p_hotkeys, p_hotkeys, sizeof(p_hotkeys) );

Now it could be that I'm missing something buried in a header file somewhere that wraps malloc() calls, checking for null returns and throwing out an error, but I don't think so. I do notice this in vlc.c:

Code: Select all

putenv( "MALLOC_CHECK_=2" );

but that is not going to catch null mallocs surely? And even the return from that isn't checked...

This is a first scan through just a few modules, and those calls simply leapt out at me. I'm wondering if this code is bomb-proof, and what other goodies await me .
The primary reason I make this point is that the application is to run on stand-alone monitors at an airport - i.e. it's very public. The last thing I need is a bunch of crashed applications with dialog boxes sat on the screen.
Can anyone allay my fears?

Dave

[Sigmund]

If you really do run out of memory that is a fatal error. Checking the return value will not make the stituation any better. It would only make the error message come from vlc and not the operating system.

[SoftwareDave]

Hmm, well that's a new way of looking at it. In the example I quoted it would mean the application writing to an illegal area of memory. Depending on the OS that would cause a core dump, GPF or maybe you could trash the system, rather than the application failing gracefully. Yes, under 32-bit OS's you have practically unlimited memory and you're unlikely to run out, but a null return from malloc can also occur with a corrupted heap.
The point I'm trying to make is that checking function calls for errors is standard good programming practice. Within minutes of me starting to look at the code I found this. Can I trust the rest of the code? Are all mallocs freed, for example?
Still worried...

[Sigmund]

I know. VLC sourcecode is in general quite clean and nice. It is reentrant and thread safe. I pushed some thousand files of random data through vlc. Total amount of memory leaked was 283 bytes. Also we do try to check most mallocs, but some times it's just too much a bother.

On modern hardware a operating system should be able to recover from GPF and not crash whatever a userspace program does. In my oppinion it doesn't make much difference if the program core dumps, is killed by the kernel or realizes that it cannot continue and quits.

All this said VLC is mostly written by students (some of them very talented), and on peoples spare time. So the code is not proof-read by independent people, and there has been little or no structured testing. (unit testing etc).

But we do have millions of users and a fairly low amount of crash reports

[SoftwareDave]

Thanks for explaining, Sigmund. Leaves me in somewhat of a dilemma, though. On the one hand this does look a good system, it does what I need it to do and I have the source code. On the other hand, I have a high profile customer (Las Vegas airport) and I will have to support this code, when I feel a little nervous about it. Don't get me wrong, I fully appreciate the time and effort that has gone into it, and I applaud the concept of open source.
Hmm...

[Gibalou]

It really seems like what you are actually looking for is a "customer supported" product where if anything goes wrong you can go back to the seller and ask them to fix the problem.
Sorry but most Open Source software don't work that way. The software doesn't come with any warranty at all.

Doesn't mean it is low quality though but if you want to use it in production then I would advise you to do your own testing to be confident it is working fine enough for your needs. If you find some problems you can also always try to ask for help here (no warranty we'll fix them but we usually try to).

Now, as for the malloc() issue, there is actually quite a controversy around it

On most operating systems nowadays, malloc() will always succeed and return a valid (virtual memory) pointer even if there is no more physical memory available (thus leading to a crash if you try to use this pointer).
Furthermore, if an application is short of memory, it will likely have no choice but to exit anyway.

Because of this, a lot of people argue that checking the return value of malloc() is not only useless but also tends to clutter the source code. I tend to agree with that, although I can think of certain cases (mainly for embedded environments) where it might still be useful to check for the return value of malloc()... but VLC is not really designed for low memory footprint environments anyway.

[SoftwareDave]

Thanks Gibalou, I do understand that there is no warranty, and I did think twice about bringing up the problem in the first place - I don't want to appear a leech or anything (Hey here's this free code! Why don't you guys make it do what I want? And for FREE!!!).
Now that you've explained the malloc issue, I think can feel more comfortable. The problem was that I took a look at a couple of source files, and the malloc calls just jumped at me. I personally couldn't, if my life depended on it, call malloc without checking the return, but I guess that's because I'm old skool and cut my teeth on DOS, Windoze 3.1 and embedded systems.
I hope you understand it made me worry:"if these aren't checked, what else isn't checked?" However, it looks like maybe I have been over-concerned. I honestly was not aware of the controversy over malloc checking.
It looks like I can confidently proceed, then. Thanks for your comments, and I'm pretty sure I'll be asking for help again!
I just hope I'll be able to contribute something back at some point.

Dave