From: Jason Green 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: 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> <7704-Fri18Aug2000203741+0300-eliz AT is DOT elta DOT co DOT il> <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" 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));