[mapserver-dev] Ticket 3559 - malloc/calloc/realloc return values should always be checked

[Stephen Woodbridge]

On 10/7/2010 9:35 AM, Frank Warmerdam wrote:
> Alan Boudreault wrote:
>> Devs,
>>
>> I'm about to begin this task and would like to see if you agree with
>> what I plan to do.
>>
>> Basicaly, the task is to pass through the code source and verify that
>> all memory allocation calls are checked. This implies, if the function
>> that contains the memory allocation wasn't already returning any error
>> value to its caller, to add checks in the callers and do the
>> appropriate action.
>
> Alan,
>
> I'd just like to go on the record and not feeling that it is important
> to check all memory allocation return values. If we get to the point
> where small allocations are failing it is unlikely that we will be able
> to recover in a meaningful way, and the error checking code may bulk stuff
> up in unhelpful ways. I say, let things segfault - it's as clean an exit
> as some others.
>
> Keep in mind our ability to actually *test* the new error cases is minimal.
>
> Just my two cents worth...
>
> Best regards,

I generally concur with this but would suggest a slightly more active 
approach.

1. check the allocation and if it fails
2. write and error to STDERR (ideally with the function name and line 
line number and then exit
3. unless there is a good change that the caller will check the failure 
AND be able to recover

The rationale for this is that the segv might happen in some place 
totally unrelated to the allocation failure and it is easier to debug if 
we die closer to the failure. I don't think this would bulk things up to 
much. Maybe the test and error can be done in a #define like

#define CHECK_ALLOC(a) if (!(a)) { fprintf(stderr, "%s: %d: Memory 
allocation failed! exiting at %s!\n", __FILE__, __LINE__); exit(1); }

-Steve W