Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup ElfLoader and BootElf #1463

Merged
merged 5 commits into from Jan 24, 2015

Conversation

phire
Copy link
Member

@phire phire commented Nov 1, 2014

Various cleanups.

I originally went in to remove a GetPointer, but this code has been hardly touched since the original Open Sourcing, so I couldn't resist cleaning up a lot of other things while I was there.


{
File::IOFile f(filename, "rb");
f.ReadBytes(mem.data(), (size_t)filesize);
File::IOFile f(filename, "rb");

This comment was marked as off-topic.

@lioncash
Copy link
Member

lioncash commented Nov 1, 2014

Other than that, looks good to me.

@phire
Copy link
Member Author

phire commented Nov 2, 2014

Indentation fixed.

@@ -210,7 +169,7 @@ bool ElfReader::LoadSymbols()
int type = symtab[sym].st_info & 0xF;
int sectionIndex = Common::swap16(symtab[sym].st_shndx);
int value = Common::swap32(symtab[sym].st_value);
const char *name = stringBase + Common::swap32(symtab[sym].st_name);
const char* name = stringBase + Common::swap32(symtab[sym].st_name);
if (bRelocate)
value += sectionAddrs[sectionIndex];

This comment was marked as off-topic.

@phire
Copy link
Member Author

phire commented Nov 2, 2014

@skidau Fixed those issues. As an added bonus, that example will now work automatically without setting a dvdroot.

else
{
DEBUG_LOG(MASTER_LOG,"Prerelocated executable");
PanicAlert("Error: Dolphin doesn't know to load a relocatable elf.");

This comment was marked as off-topic.

This comment was marked as off-topic.

@waddlesplash
Copy link
Contributor

@phire ping. there are merge conflicts now.

@phire
Copy link
Member Author

phire commented Nov 17, 2014

Great, I'll clean those up.
On 18/11/2014 8:18 AM, "Augustin Cavalier" notifications@github.com wrote:

@phire https://github.com/phire ping. there are merge conflicts now.


Reply to this email directly or view it on GitHub
#1463 (comment).

@JMC47
Copy link
Contributor

JMC47 commented Dec 3, 2014

Any reason to not merge this?

@delroth
Copy link
Member

delroth commented Dec 26, 2014

@phire any progress on reproducing and fixing the crash that @skidau was reporting?

 * The file already exsists, otherwise we wouldn't have gotten
   this far in the boot.
 * We have already checked if it's a Wii or GameCube elf,
   besides, it's too late to change our minds now anyway.
 * On Wii - Don't call EmulatedBS2, it can never succeed as
   it knows nothing about booting elfs. Just call the
   SetupWiiMemory directly if needed.
 * On GameCube - We still call EmulatedBS2_GC, but we stop
   it from running Apploader, which might boot something
   unexpected from the default iso or DVD root folder.
 * Don't claim to support any features we don't, like relocation
 * Actually zero-out BSS sections, as memory might not be already
   zeroed.
 * Deleted commented out code.
 * Removed GetPointer, updated to more modern interface methods.
 * Updated pointer types style from "u32 *x" to "u32* x"
Instead of swaping each word of the elf code section(s) looking
for a match to our pattern, we swap the pattern just once (at
compile  time) and test against our swapped pattern.
@phire
Copy link
Member Author

phire commented Jan 24, 2015

Rebased. This can now be merged.

delroth: the crash was fixed.

EmulatedBS2_GC(true);

Load_FST(_StartupPara.bWii);
if(!Boot_ELF(_StartupPara.m_strFilename)) {

This comment was marked as off-topic.

This comment was marked as off-topic.

Also fix mistake in error message.
@phire
Copy link
Member Author

phire commented Jan 24, 2015

Braces fixed.

Sonicadvance1 added a commit that referenced this pull request Jan 24, 2015
@Sonicadvance1 Sonicadvance1 merged commit d544cc3 into dolphin-emu:master Jan 24, 2015
@phire phire deleted the cleanup_ELFloader branch January 24, 2015 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
7 participants