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

[Alan Boudreault]

Hi devs,

Through my work on this task, I realized that the *Small* use is very 
relative. What is considered a small alloc these days? Most of the time, the 
allocations are made dynamically, so it is hard to see if it's a small alloc 
or not. Currently, when I'm not sure if it's a small alloc or not, I use a 
macro that I define: MS_CHECK_ALLOC, which check the pointer, call msSetError 
and return a failure code. Of course, I have to check if this return code is 
properly checked if the function wasn't returning a failure code before. I 
think there will be a lot more MS_CHECK_ALLOC uses than our msSmall* funtions. 
At this point, I have 2 options:

* Continue that way to allow MapServer to return an error message to the user  
as most as possible.

* Replace msSmallMalloc by msMalloc and use it almost everywhere. 
 
regards,
Alan

On November 17, 2010 02:24:03 pm Lime, Steve D (DNR) wrote:
> I guess I favor the explicit names (should msStrdup be msSmallStrdup?),
> less confusion. If I saw msMalloc() I would assume it's equivalent to
> malloc() in terms of use (like msFree/free) and that's not the intention.
> I don't think there's a need for a vote. The discussion has been out in
> the open and others have had the chance to weigh in. It would be nice to
> see this documented in the context of do's and don'ts related to the
> cleanup work Alan did. We don't want to go and reintroduce the vary code
> he worked so hard to clean up. Perhaps a wiki page?
> 
> Steve
> 
> -----Original Message-----
> From: mapserver-dev-bounces at lists.osgeo.org
> [mailto:mapserver-dev-bounces at lists.osgeo.org] On Behalf Of Daniel
> Morissette Sent: Tuesday, November 16, 2010 2:46 PM
> To: mapserver-dev at lists.osgeo.org
> Subject: Re: [mapserver-dev] Ticket 3559 - malloc/calloc/realloc return
> values should always be checked
> 
> Reviving an old thread... it seems that several people were in support
> of this option, but some were concerned about the proliferation of
> exit() and the side-effects that they could create in some non-CGI
> environments. While I would agree that spawn-fcgi is flawed if it cannot
> restart aborted processes, especially in cases where a process is
> running out of memory, I also believe that we could try to be more
> exit-friendly in some non-fatal situations and this can be discussed
> further in the context of ticket #3099.
> 
> So for the short term, it would seem to make sense to introduce the use
> of msSmallMalloc(), msSmallCalloc() and msStrdup() which would exit() on
> out of memory after attempting to report the error through stderr, since
> there is not much we can do anyway when we're out of memory.
> 
> With respect to better handling of exit() in other situations, let's
> save that for another thread.
> 
> Do we need a PSC vote before we go ahead with msSmallMalloc(),
> msSmallCalloc() and msStrdup()?
> 
> What about using the names msMalloc() and msCalloc() instead and
> properly documenting that they are intended for use with small
> allocations only (as is done for GDAL's CPLMalloc family of functions)?
> 
> Daniel
> 
> Frank Warmerdam wrote:
> > strk wrote:
> >> On: Thursday, October 07, 2010 9:42 AM, Frank Warmerdam wrote:
> >>> I would like to propose an msSmallAlloc() function that behaves in a
> >>> similar
> >>> fashion, just writing an error to stderr in case of failure and calling
> >>> exit().  Likewise, an msStrdup() with similar behavior.
> >> 
> >> Better than nothing, but this will fail to be catchable by mapscript
> >> or the CGI (think XML or in-image exception).
> >> It's very frustrating when you get a blank image and have
> >> to resort to forcing text/html content type or even worst
> >> reading server logs, and there are already lots of such cases.
> > 
> > Strk,
> > 
> > Well, I can assure you that in the situation where msSmallAlloc()
> > fails there would be no way of actually preparing an error report
> > and burning it into an image for "inimage" error reports.  Things
> > are already too desperate.
> > 
> > It is in fact very doubtful that we could even allocate a new
> > errorObj for the error stack if we called msSetError().
> > 
> > To put all this in context, GDAL/OGR has always "just died" with an
> > error message to stderr if small allocations fail, including when run
> > in the context of applications like ArcGIS.  This has never yet, as
> > far as I recall, been reported as a problem.
> > 
> > Best regards,

-- 
Alan Boudreault
Mapgears
http://www.mapgears.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osgeo.org/pipermail/mapserver-dev/attachments/20101123/7a900969/attachment-0001.html