On Wed, 2012-07-18 at 16:17 -0700, Roland McGrath wrote:
This seems vaguely reasonable to me, but OTOH since in the mmap case
we do
eagerly set up the shdr pointers in file_read_elf, I think it makes sense
to be consistent and do that in __libelf_readall too. It should also save
a little memory to not malloc the ehdr and shdr copies. That means that
__libelf_readall really ought to free the old ehdr copy (which is always
cached) and the shdr copies if they were cached before. Most of the code
can just be broken out of file_read_elf into a subroutine,
e.g. __libelf_maybe_precache_headers. __libelf_readall will just need to
free the old state and clear the shdr_malloced flag.
I thought about that, but was afraid about other (concurrent) users of
the already allocated data if we freed it and swapped in a new mmaped
version. So I decided to just keep what we have and only copy in the new
shdr data in load_shdr_wrlock () when requested. Do you think the memory
savings are significant enough to risk data races? It can probably be
done safely, but does require careful analysis for IMHO little gain.
Thanks,
Mark