Refactor pwndbg.commands.windbg.dX and pwndbg.hexdump.hexdump #1341
Conversation
|
@ruijia-zhou, try running |
|
Thanks for the information. Sorry for this. I am trying now. |
Codecov Report
@@ Coverage Diff @@
## dev #1341 +/- ##
==========================================
- Coverage 54.87% 54.81% -0.06%
==========================================
Files 153 153
Lines 19384 19423 +39
Branches 1815 1823 +8
==========================================
+ Hits 10636 10646 +10
- Misses 8298 8328 +30
+ Partials 450 449 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@gsingh93 thanks a lot for the information. Just passed all checks. |
pwndbg/commands/windbg.py
Outdated
|
|
||
| if not to_string: | ||
| if not to_string and lines != []: |
There was a problem hiding this comment.
nit: the "Pythonic" way to do this is if not to_string and lines: https://stackoverflow.com/questions/53513/how-do-i-check-if-a-list-is-empty
I'd also accept len(lines) != 0, just because it'll catch type issues where lines is not a list (even though this isn't considered "Pythonic").
There was a problem hiding this comment.
Thanks for the advice, I will do it now
pwndbg/hexdump.py
Outdated
|
|
||
| color_scheme = None | ||
| printable = None | ||
|
|
||
|
|
||
| def get_type(size): |
There was a problem hiding this comment.
Could you move this function into pwndbg.gdblib.typeinfo? I think it makes more sense there and we probably want to use it in other places in the codebase.
There was a problem hiding this comment.
Sure, I will do it now :)
pwndbg/hexdump.py
Outdated
| address = int(address) & pwndbg.gdblib.arch.ptrmask | ||
| count = int(count) | ||
|
|
||
| type = get_type(size) |
There was a problem hiding this comment.
type is a built-in Python function. In the future we'll have a linter preventing names like this, so could you rename the variable to something else? If type really is the best name for this, we can go with something like type_.
pwndbg/hexdump.py
Outdated
|
|
||
| else: | ||
|
|
||
| """ |
There was a problem hiding this comment.
Nit, generally these types of string comments only go at the top of a function, so in this case we should convert it to a normal comment with #.
pwndbg/hexdump.py
Outdated
| for i in range(count): | ||
| try: | ||
| gval = pwndbg.gdblib.memory.poi(type, address + i * size) | ||
| # print(str(gval)) |
There was a problem hiding this comment.
Let's remove all the debugging code before merging this.
pwndbg/hexdump.py
Outdated
| rows = [values[i * row_sz : (i + 1) * row_sz] for i in range(n_rows)] | ||
| lines = [] | ||
|
|
||
| # sys.stdout.write(repr(rows) + '\n') |
There was a problem hiding this comment.
Same note about the debugging code.
|
Hi, @gsingh93 ,I have revised the code. Please have a look. Thanks! |
Replace the implementation of pwndbg.commands.windbg.dX with pwndbg.hexdump.hexdump, see issue #1280. The code in dX has been moved to dexdump. Some optional options have been added to the function signature of hexdump such that dX is implemented using a single call to hexdump.