From: Jason Green <news AT jgreen4 DOT fsnet DOT co DOT uk>
Newsgroups: comp.os.msdos.djgpp
Subject: Re: Symify crash
Date: Sat, 19 Aug 2000 23:18:42 +0100
Organization: Customer of Energis Squared
Lines: 58
Message-ID: <vd1ups40msfq54olcubqgutg1n2umqu6vq@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>
NNTP-Posting-Host: modem-52.depacon.dialup.pol.co.uk
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
X-Trace: news6.svr.pol.co.uk 966723541 18298 62.136.88.52 (19 Aug 2000 22:19:01 GMT)
NNTP-Posting-Date: 19 Aug 2000 22:19:01 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:

> Thanks for catching this, this is indeed a bug.

>  (In the v1.x days, it worked because
> malloc returned zeroed-out storage.)

That's interesting to know.

> > Also, ISTR that generally accepted best practice is *not* to cast the
> > return from malloc() so the code should read:
> > 
> > syms = malloc(num_syms * sizeof(SymNode));
> 
> Yes, but that's not really a bug.  Casting malloc doesn't produce
> invalid code, it just obscures possible problems from not including
> stdlib.h, which in this case *is* included.

Very true.  It's more a style issue, and scanning through the code
suggests this may be the least of the problems. ;-/

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.  Or replace with calloc(), since, as you
say, the code was originally written when malloc() zeroed-out memory.


I won't pretend to understand what any of the code does, but scanning
through syms.c, I see other potential bugs:

In the function process_coff(), this code (line 188) appears to have
been fixed to zero-out f_string_table:

  fread(&strsize, 1, 4, fd);
  f_string_table = (char *)malloc(strsize);
  /* CWS note: must zero or pukes below.  Does not fill from file. */
  memset(f_string_table,0,strsize);
  fread(f_string_table+4, 1, strsize-4, fd);
  f_string_table[0] = 0;

But in the little used process_aout(), some similar code (line 324)
has not been fixed:

  fread(&string_table_length, 1, 4, fd);
  f_string_table = (char *)malloc(string_table_length);
  fread(f_string_table+4, 1, string_table_length-4, fd);
  f_string_table[0] = 0;


Also, the code in process_coff() that corresponds to the memset bug in
process_aout(), does not even use memset() at all, could this be worth
adding? line 218:

  files = (FileNode *)malloc(num_files * sizeof(FileNode));

  syms = (SymNode *)malloc(num_syms * sizeof(SymNode));