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

read/archive: locate members by table in AIX archive #467

Merged
merged 4 commits into from
Sep 30, 2022

Conversation

ecnelises
Copy link
Contributor

This is from review of #462. Iterating over members by member table can avoid potential infinite loop if the file is malformed. Also, this change sets names for a parsed file.

Copy link
Contributor

@philipc philipc left a comment

Choose a reason for hiding this comment

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

Please also run cargo fmt so that CI passes.

src/read/archive.rs Outdated Show resolved Hide resolved
src/read/archive.rs Outdated Show resolved Hide resolved
src/read/archive.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@philipc philipc left a comment

Choose a reason for hiding this comment

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

Can you fix the cargo fmt in src/archive.rs as well please? I think it's from the previous PR (#462).

@philipc philipc merged commit 89eec0a into gimli-rs:master Sep 30, 2022
let table_size = parse_u64_digits(&header.size, 10)
.read_error("Invalid AIX big archive member size")?;
let members_count = data
.read_bytes_at(member_table_offset, 30)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this have been a length of 20?

I'll do a followup PR for some other things, so I'll change this if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, it should be 20, Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants