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.

Sign up for GitHub

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

Jump to bottom

Code Refactoring for the Dumpulator Package #47

Merged
merged 31 commits into from
Feb 25, 2023
Merged

Code Refactoring for the Dumpulator Package #47

merged 31 commits into from
Feb 25, 2023

Conversation

anthonyprintup
Copy link
Contributor

@anthonyprintup anthonyprintup commented Feb 24, 2023 •
edited
Loading

Currently the project consists of a large codebase which at first glance isn't friendly to new developers. I've made some changes in an attempt to make the code more easily readable/understandable, and correct (in cases where bugs have been found).

Applicable scopes:

  • details
  • dumpulator
  • handles
  • memory
  • modules
  • native

@anthonyprintup
refactor(dumpulator): Removed comments containing unused code in the …
f2b665a 
…`LazyPageManager` class
@anthonyprintup
refactor(dumpulator): Refactored LazyPage to use dataclasses
be72eaa
@anthonyprintup
refactor(dumpulator): Refactored UnicornPageManager to use dataclasses
29494d2
@anthonyprintup
refactor(dumpulator): Refactored ExceptionInfo to use dataclasses
b69a9f3
@anthonyprintup
refactor(dumpulator): Refactored LazyPageManager to use dataclasses
5edb927
@anthonyprintup
refactor(dumpulator): Refactored SimpleTimer's initializer to bette…
a349365 
…r reflect the type of `time`, which should be assigned a float
@anthonyprintup
refactor(dumpulator): Simplified the chain comparison in _hook_mem
9f931f2
@anthonyprintup
squash! refactor(dumpulator): Refactored ExceptionInfo to use datac…
e395d9d 
…lasses
@anthonyprintup
fix(dumpulator): Fixed an issue in Dumpulator.call where the defaul…
12707a4 
…t argument values were mutable
@anthonyprintup
refactor(dumpulator): Refactored Dumpulator.map_module
5b3cdda 
Refactored potentially ambiguous code when setting execute permissions on a page.
@anthonyprintup
style(dumpulator): Removed empty lines
e762ca2
@anthonyprintup
refactor(dumpulator): Refactored _hook_invalid
23f353c 
Removed unreachable code.
@anthonyprintup
refactor(dumpulator): Refactored possible static methods
d4bebb7
@anthonyprintup
refactor(dumpulator): Removed unused variable in `Dumpulator._setup_m…
817134d 
…emory`
@anthonyprintup
refactor(dumpulator): Refactored _unicode_string_to_string
c9a10ee
@anthonyprintup
refactor(dumpulator): Changed the underlying type of `ExceptionInfo.c…
e50631f 
…ode_hook_h`
@anthonyprintup
refactor(dumpulator): Refactored handles.py
0e8e873 
Fixed type hints and built-in shadowing.
@anthonyprintup
fix(dumpulator): Fixed mutable default argument values in handles.py
454e510
@anthonyprintup
refactor(dumpulator): Small refactors in memory.py
3b1c7ed 
Removed unused imports, renamed arguments which were shadowing built-in names, simplified chained comparisons, fixed typos.
@anthonyprintup
fixup! refactor(dumpulator): Small refactors in memory.py
8ee4bd7
@anthonyprintup
refactor(dumpulator): Refactored classes in memory.py
9eeac5e 
Converted classes into dataclasses.
@anthonyprintup
refactor(dumpulator): Refactored classes in modules.py
34dca4c 
Converted classes into dataclasses.
@anthonyprintup
refactor(dumpulator): Refactored native.py
f378cdd
@anthonyprintup
style(dumpulator): Added new line to the end of file in details.py
885f75d
@anthonyprintup
Copy link
Contributor Author

As a note, I think it would be a good idea to get rid of typing imports for standard collections (refer to this and PEP 585).

@anthonyprintup anthonyprintup marked this pull request as ready for review February 24, 2023 05:43
@anthonyprintup
fix(dumpulator): Fixed a bug in the Module class
51da004 
Converting the class into a dataclass introduced a bug which made the class unhashable.
Added a `__hash__` method to avert this issue.
@anthonyprintup
fix(dumpulator): Fixed a keyword argument error
9dfa77f
@anthonyprintup
fix(dumpulator): Added missing class variables to the Module class
bb6e856
mrexodia
mrexodia requested changes Feb 24, 2023
View reviewed changes
Copy link
Owner

@mrexodia mrexodia left a comment

Choose a reason for hiding this comment

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

Mostly good, appreciate the work. Just some minor things I don't agree with.

src/dumpulator/dumpulator.py Outdated Show resolved Hide resolved
src/dumpulator/dumpulator.py Outdated Show resolved Hide resolved
src/dumpulator/dumpulator.py Outdated Show resolved Hide resolved
src/dumpulator/memory.py Show resolved Hide resolved
src/dumpulator/memory.py Outdated Show resolved Hide resolved
src/dumpulator/memory.py Outdated Show resolved Hide resolved
src/dumpulator/modules.py Show resolved Hide resolved
src/dumpulator/modules.py Show resolved Hide resolved
@mrexodia mrexodia merged commit cb01275 into mrexodia:main Feb 25, 2023
@mrexodia
Copy link
Owner

Thanks a lot!

@anthonyprintup anthonyprintup deleted the refactor branch February 25, 2023 14:24
@anthonyprintup anthonyprintup mentioned this pull request Feb 25, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrexodia mrexodia Awaiting requested review from mrexodia

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@anthonyprintup @mrexodia