On Thu, 2012-07-19 at 11:56 +0200, Mark Wielaard wrote: > 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.
Also note the somewhat related item in the TODO file: ** shdrs in read-only files When reading (ELF_C_READ*) then there is no need to malloc Shdr structure in elfXX_getshdr if file is mmaped and unaligned access is allowed or the structure is aligned. Use ELF_F_MALLOCED flag to differentiate. _______________________________________________ elfutils-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/elfutils-devel
