Use u64, even on 32-bit to support reading elf-64 files. #34
Conversation
|
Do you have a use case in mind? I'm not sure when this support would be necessary. |
|
Looks good, not sure why the flags length has to be changed though. |
|
And in the context of running this in a kernel when reading multiboot, I don't think you can ever have a mismatch. But I'd like to have more tests for reading x86 and x86_64 and if you test reading a 64 bit multiboot section on a 32 bit os, you might end up failing. The other scenario is reading elf data for a 64 bit binary on a 32 bit system. Granted, this code isn't used in that scenario currently but it'd be nice to support it if the code ever allows for it. The only downside is that 32 bit users of the code end up with 64 bit integers where the top 4 bytes are always 0. |
src/elf_sections.rs
Outdated
| fn addr(&self) -> usize { | ||
| self.addr as usize | ||
| fn addr(&self) -> u64 { | ||
| self.addr.into()// as u64 |
There was a problem hiding this comment.
Nitpick: Looks like a leftover comment from development
src/elf_sections.rs
Outdated
| fn size(&self) -> usize { | ||
| self.size as usize | ||
| fn size(&self) -> u64 { | ||
| self.size.into()// as u64 |
There was a problem hiding this comment.
Leftover comment from development
|
LGTM. I only had a couple minor nitpicks regarding unnecessary comments |
@jmi2k @robert-w-gries
The current code mostly assumes that you're reading ELF headers which match the size of your platform, due to the use of
usize. That means that the code is always memory safe but potentially truncating data. This proposes always returningu64and promoting allu32types to that, which means you always get all of the data.What do you think?