Cleanup ElfFileIdentifierForMapping code in linux_dumper.cc

[GoogleCodeExporter]

Short version:
ElfFileIdentifierForMapping is NOT idempotent as the name and its signature 
suggests. It mangles the mapping path name and does that in a gross way.

Full version:
Currently ElfFileIdentifierForMapping() in 
client/linux/minidump_writer/linux_dumper.cc has a very intricate logic.

1. The method signature takes both a const MappingInfo& and a mapping_id. At 
first glance this might seem redundant, since a MappingInfo can be retrieved 
from the |mappings_| list by its id. However this seems to deal with the case 
of Mappings coming from *somewhere else* (?) and not part of the app's mappings 
list. Eventually, at this point, this could take only a ref to MappingInfo ... 
but (see next points). (Introduced in http://breakpad.appspot.com/242001)

2. The name ElfFileIdentifierForMapping() would suggests that the function is 
idempotent and just retrieves an identifier given a mapping, especially 
considering that it takes a *const* reference to MappingInfo. However, at 
current state, it can alter the |name| of a mapping (to strip out the " 
(deleted)" part from the pathname). This is extremely confusing.


3. There are call sites that depend on this behavior 
(ElfFileIdentifierForMapping cleaning up the path name). When cleaning up 
ElfFileIdentifierForMapping, those call sites should be refactored as well.

4. The way the "(deleted)" part of the path name is deleted is a bit gross:  
HandleDeletedFileInMapping() returns the cleaned up |filename|. However, 
ElfFileIdentifierForMapping never looks into that returned |path|, rather it 
just speculates on its
implementation: when filename_modified == true, it assumes it did truncate the 
path of exactly (sizeof(kDeletedSuffix) + 1) characters. Hence, the code in 
ElfFileIdentifierForMapping (~line 136) adds a null-terminator on the basis of 
such speculation @ filename_len -  sizeof(kDeletedSuffix) + 1. (Introduced in 
https://breakpad.appspot.com/228001)


More discussion in https://breakpad.appspot.com/7734002/

Original issue reported on code.google.com by primi...@chromium.org on 16 Oct 2014 at 9:42