From: Jason Green <news AT jgreen4 DOT fsnet DOT co DOT uk>
Newsgroups: comp.os.msdos.djgpp
Subject: Re: Symify crash
Date: Tue, 22 Aug 2000 00:32:18 +0100
Organization: Customer of Energis Squared
Lines: 59
Message-ID: <6ve3qscbmtuk1ue33c57ldhmvp59ebnbit@4ax.com>
References: <3 DOT 0 DOT 5 DOT 32 DOT 20000813122244 DOT 007c0c00 AT pop DOT mail DOT yahoo DOT com> <3405-Sun13Aug2000163046+0300-eliz AT is DOT elta DOT co DOT il> <3996E3F4 DOT C9B37F8B AT softhome DOT net> <2593-Sun13Aug2000214525+0300-eliz AT is DOT elta DOT co DOT il> <fadqpscd8dj2q2la9jbojdvuou9a9283vl AT 4ax DOT com> <7704-Fri18Aug2000203741+0300-eliz AT is DOT elta DOT co DOT il> <m91rps83eoo8thnrmjumrbg1i9fhdlbf8a AT 4ax DOT com> <1438-Sat19Aug2000213809+0300-eliz AT is DOT elta DOT co DOT il> <drutpscncl1imai9btj7k697natrpbtnqr AT 4ax DOT com> <7263-Sun20Aug2000090732+0300-eliz AT is DOT elta DOT co DOT il>
NNTP-Posting-Host: modem-26.desitin.dialup.pol.co.uk
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
X-Trace: newsg3.svr.pol.co.uk 966900764 16595 62.136.89.26 (21 Aug 2000 23:32:44 GMT)
NNTP-Posting-Date: 21 Aug 2000 23:32:44 GMT
X-Complaints-To: abuse AT theplanet DOT net
X-Newsreader: Forte Agent 1.7/32.534
To: djgpp AT delorie DOT com
DJ-Gateway: from newsgroup comp.os.msdos.djgpp
Reply-To: djgpp AT delorie DOT com

"Eli Zaretskii" <eliz AT is DOT elta DOT co DOT il> wrote:

> > If the code is to be patched, do you think it would be worth doing a
> > global replace on all the malloc() calls?
> > 
> > Either change them to xmalloc(), as is done in symify.c, since returns
> > are never checked for NULL.
> 
> If you would like to work on this, please do.  Thanks.

Certainly, I can produce a patch to replace the malloc() calls if it
is agreed that this would be worthwhile.  

I am wary of publishing any mods to the code, which I must admit is
mostly cryptic to me, in case something breaks.  I don't have the
skill (or time) to do thorough testing, and symify has never crashed
here either.

My observations, however, are that the code makes many assumptions
about the successful return of system calls, mostly malloc(), fread()
and fseek(), and that these should be tied down.

I would propose two patches.  Neither will fix any immediate problem,
though bugs should be easier to track down as a consequence:

1) replace malloc with either xmalloc or calloc (I am still not
convinced about this BTW).

2) introduce assert's everywhere it seems that successful file
operations are assumed.  I think it's too dangerous for me to make
fixes more than this, in case of false positives.

> > Or replace with calloc(), since, as you
> > say, the code was originally written when malloc() zeroed-out memory.
> 
> No, this is actually not a good idea, and I'm sure whoever wrote that
> stuff didn't use calloc for a good reason: 

That is not bourne out by the comments from Charles Sandmann, AIUI.

> calling calloc causes the
> DPMI server to page in all the memory, in order to initialize it.
> This could be a performance hit for large programs where the symbol
> table is several MBytes long.  

There is a performance hit, but would it be noticeable in this case?

> It is also unnecessary, since the code
> reads in the data into the malloc'ed buffers right away, and the data
> *must* by definition fill the entire buffer, because the size of the
> data was read from the COFF headers.

Except that the file reads are not checked to see if the entire buffer
was filled.  Zeroing the buffers would at least allow some consistency
in future crashes so that bugs could be caught.

[Feel free to take this to djgpp-workers if that is more appropriate
but please bear in mind I don't subscribe to that list.  Also, it
might be a few days before I am free to reply to any responses].