philippe44 / AirConnect

Use AirPlay to stream to UPnP/Sonos & Chromecast devices
Other
3.55k stars 218 forks source link

Completion of error handling #129

Closed elfring closed 5 years ago

elfring commented 5 years ago

Would you like to add more error handling for return values from functions like the following?

philippe44 commented 5 years ago

In summary no, but I'm happy to explain why. As you probably know, the handling of malloc error has been the source of lots of debates, calling names and endless discussions. I've personally settled on a logic that uses the following considerations

1- Is your system memory-constrained, embedded or is it a rich host OS 2- Is your application mission-critical or simply "just an app" 3- Are you buiding a library or an application 4- How much is the malloc failure manageable / recoverable (how "early")

In an embedded, mission-critical system, don't use memory allocation, and if you have to, don't use heap-based allocation. If you still decide to, at least handle all allocation failures and make an informed decision of restarting or trying to recover In a rich host OS and if your application is "just an app", don't handle the error unless it happens early enough in a "task" where you can simply abort the "task" without compromising the whole application. An un-handled error will cause such non-critical application to crash, so have a monitoring solution to restart it. But don't bloat the code of such simple application with malloc failure management At the end of the day, for "just an app" on a rich OS, if a malloc of 1kB is not satisfied, it means your system has much more serious problems than your little application, so just let it crash which will happen at the first use of the pointer. Now, in the case or AirConnect, I would manage a malloc failure of a large chunk of memory that happens while creating a player, as long as the result is simply that the player will not be created and the memory request is large enough so that I can resonnabily think that other smaller allocations for existing players will still be satifisied

I wish I were always fully compliant with these criterias, but I'm not, candidely. Sometimes I'm lazy and I handle a malloc failure that I should not :-) but when I'm modifying a library that comes from somebody else, I always respect his style. If the owner is handling all memory allocation failures, I would do the same in his modified code, even if I disagree.

When building libraries, I also handle things differently because they can be embedded in different systems, I don't know in advance. So the rule should be "no malloc" but if you have to, wrap them into something that let the user do what he wants and try to manage errors, don't do something that would lead to a crash.

elfring commented 5 years ago

How do you think about to improve static source code analysis also for this software?

philippe44 commented 5 years ago

Ok - you sound like a troll or a bot. I'll give you one chance to make a proper response and then I'll close that issue