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

[Alan Boudreault]

Devs, 

I've committed my changes in svn. I wrote a small coding guidelines page in 
the wiki about memory allocations and string manipulations. Feel free to 
improve it if it is missing anything:

 http://trac.osgeo.org/mapserver/wiki/CodingGuidelines

regards,
Alan

On November 17, 2010 02:30:25 pm Alan Boudreault wrote:
> Yes, I'll take care to create a wiki page after the work.
> 
> 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