Mail Archives: djgpp/2000/08/21/19:45:23
"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].
- Raw text -