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

PE: Change Section Table Real Name Handling #438

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/pe/section_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ impl SectionTable {
table.characteristics = bytes.gread_with(offset, scroll::LE)?;

if let Some(idx) = table.name_offset()? {
table.real_name = Some(bytes.pread::<&str>(string_table_offset + idx)?.to_string());
if let Ok(real_name) = bytes.pread::<&str>(string_table_offset + idx) {
table.real_name = Some(real_name.to_string());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it would be more idiomatic to do:

table.real_name = bytes.pread::<&str>(string_table_offset + idx).ok().map(String::to_owned);

the real question is whether:

  1. It should be considered a fatal error if the name is not utf8
  2. we should log on error, in which case keeping the if let Ok is better

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry it took me so long to reply!
As far as I understand, real_name is stored in COFF String Table, but there is nothing stated about the encoding. However? for Symbol Name the following is stated:

By convention, the names are treated as zero-terminated UTF-8 encoded strings.

Maybe, it can be applied to real_name, too?

However, I do not consider it to be a fatal error, so maybe we should stick to if let Ok?

}
}
Ok(table)
}
Expand Down
Loading