Skip to content

Intel Hex format unpacker/packer#349

Merged
rbs-jacob merged 9 commits into
redballoonsecurity:masterfrom
EdwardLarson:feature/ihex_unpacker
Aug 3, 2023
Merged

Intel Hex format unpacker/packer#349
rbs-jacob merged 9 commits into
redballoonsecurity:masterfrom
EdwardLarson:feature/ihex_unpacker

Conversation

@EdwardLarson
Copy link
Copy Markdown
Contributor

@EdwardLarson EdwardLarson commented Jul 24, 2023

One sentence summary of this PR (This should go in the CHANGELOG!)
Add Identifier, Unpacker, Packer for Intel Hex format

Link to Related Issue(s)

Please describe the changes in your request.

Anyone you think should look at this, specifically?

@EdwardLarson EdwardLarson marked this pull request as draft July 24, 2023 17:37
@EdwardLarson EdwardLarson marked this pull request as ready for review July 26, 2023 16:12
@EdwardLarson EdwardLarson requested a review from rbs-jacob August 3, 2023 15:15
Copy link
Copy Markdown
Member

@rbs-jacob rbs-jacob left a comment

Choose a reason for hiding this comment

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

It probably makes sense to change the packer and unpacker to become two packers and two unpackers.

Right now both have two distinct code paths that case on tags. This is exactly what separate components are for, and I don't see the benefit to having those if statements each be inside one packer/unpacker instead of having the cases broken up across different packers and unpackers.

Comment thread ofrak_core/ofrak/core/ihex.py Outdated
Comment thread ofrak_core/ofrak/core/ihex.py Outdated
@rbs-jacob rbs-jacob changed the title Intel Hex formar unpacker/packer Intel Hex format unpacker/packer Aug 3, 2023
@rbs-jacob rbs-jacob merged commit 5b59704 into redballoonsecurity:master Aug 3, 2023
@EdwardLarson EdwardLarson mentioned this pull request Aug 10, 2023
1 task
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.

3 participants