Port `c` commands to newshell #1854

DMaroo · 2021-10-18T20:26:19Z

cv, cV and cg haven't been ported yet

Your checklist for this pull request

I've read the guidelines for contributing to this repository
I made sure to follow the project's coding style
I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
I've added tests that prove my fix is effective or that my feature works (if possible)
I've updated the documentation and the rizin book with the relevant information (if needed)

Detailed description

To port:

cv (implemented in cb)
cV (implemented in ca)
cg (to be removed)

Test plan

Add tests for new commands. Test coverage for c commands is not very good.

Closing issues

Tracking issue: #1342

librz/core/cmd/cmd_cmp.c

XVilka · 2021-10-19T04:55:37Z

librz/core/cmd/cmd_cmp.c

+	if (b) {
+		memset(b, 0xff, core->blocksize);
+		rz_io_read_at(core->io, addr, b, core->blocksize);
+		rz_print_hexdiff(core->print, core->offset, core->block, addr, b, core->blocksize, col);


@wargio please take a look at these two functions, since you wrote that.

librz/core/cmd/cmd_cmp.c

ret2libc

As you noticed, it seems this whole file is not very tested... You can see the coverage on https://app.codecov.io/gh/rizinorg/rizin/blob/6da244eca72902c3939f1ecc5a1b566c93e4b415/librz/core/cmd/cmd_cmp.c . I think adding a basic test for each command would be great.

librz/core/cmd_descs/cmd_cmp.yaml

librz/core/cmd/cmd_cmp.c

* `cv`, `cV` and `cg` haven't been ported yet

* Rename `__core_cmp_bits` to `core_cmp_bits`

* Also, simplify `cd` command handler

XVilka · 2021-10-26T05:47:05Z

[XX] db/cmd/bitmask bitmask cmp data
RZ_NOPLUGINS=1 rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -N -Qc 'b 16
wx 414243
e scr.null=1
cx 41..43 2>/dev/null
e scr.null=0
?! ?e thats fine
e scr.null=1
cx 41..11 2>/dev/null
e scr.null=0
?! ?e thats wrong
' =
-- stdout
--- expected
+++ actual
@@ -1,1 +1,0 @@
-thats fine

-- exit status: -1
[XX] db/cmd/data cmp data
RZ_NOPLUGINS=1 rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -N -Qc 'b 16
wx 41
e scr.null=1
cx 41 2>/dev/null
e scr.null=0
?! ?e thats fine
e scr.null=1
cx 11 2>/dev/null
e scr.null=0
?! ?e thats wrong
' =
-- stdout
--- expected
+++ actual
@@ -1,1 +1,0 @@
-thats fine

-- exit status: -1
[XX] db/cmd/cmd_c cmd_c*
RZ_NOPLUGINS=1 rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -N -Qc 'c* 1234567890' bins/elf/ls
-- stdout
--- expected
+++ actual
@@ -1,11 +1,0 @@
-wx 31 @ 0x00005ae0
-wx 32 @ 0x00005ae1
-wx 33 @ 0x00005ae2
-wx 34 @ 0x00005ae3
-wx 35 @ 0x00005ae4
-wx 36 @ 0x00005ae5
-wx 37 @ 0x00005ae6
-wx 38 @ 0x00005ae7
-wx 39 @ 0x00005ae8
-wx 30 @ 0x00005ae9
-wx 00 @ 0x00005aea

-- stderr
Command 'c*' does not exist.

Also forgotten e part in:

 [XX] db/cmd/regexp "/e /t\wst\d\d\d\s\w\w/i" - oldshell
RZ_NOPLUGINS=1 rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -N -Qc 'cfg.oldshell=true
w "test123 ab"
w "Test123 ab" @444
?e
e search.in=block
b 777
"/e /t\wst\d\d\d\s\w\w/i"
' malloc://1024
-- stdout
--- expected
+++ actual
@@ -1,3 +1,0 @@
-
-0x00000000 hit0_0 "test123 ab"
-0x000001bc hit0_1 "Test123 ab"

-- stderr
Command 'cfg.oldshell=true' does not exist.

DMaroo · 2021-10-26T18:03:54Z

Thank you for pointing out. However, I will fix the errors once the porting is complete.

* Add `rz_cmp.h` and `cmp.c` * Break `cmd_cmp_disasm` into 2 API functions, one for getting the diff, other for printing the diff * Refactor `cmd_cmp.c` to use the new cmd API functions

* Refactor handlers to use the new functions * Add data-data, data-str compare function, and a general print function in the `cmd` API

librz/core/cmd/cmd_cmp.c

librz/core/cmd/cmd_debug.c

librz/core/cmd/cmd_cmp.c

ret2libc · 2021-11-23T13:25:04Z

@DMaroo I think you messed up with the branch because you ended up including some changes done by @thestr4ng3r and if you look at the diff right now there are also changes from him related to DYLD.

DMaroo · 2021-11-23T13:32:24Z

Yes. My bad. Moving commits from this branch to the dbl PR might have caused it. I'll fix them.

XVilka

Please fix the remaining tests and we are good to go, IMHO:

[XX] db/cmd/data cmp data
RZ_NOPLUGINS=1 rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -N -Qc 'b 16
wx 41
e scr.null=1
cx 41 2>/dev/null
e scr.null=0
?! ?e thats fine
e scr.null=1
cx 11 2>/dev/null
e scr.null=0
?! ?e thats wrong
' =
-- stdout
--- expected
+++ actual
@@ --1,3 +-1,4 @@
 thats fine
+thats wrong

[XX] db/cmd/bitmask bitmask cmp data
RZ_NOPLUGINS=1 rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -N -Qc 'b 16
wx 414243
e scr.null=1
cx 41..43 2>/dev/null
e scr.null=0
?! ?e thats fine
e scr.null=1
cx 41..11 2>/dev/null
e scr.null=0
?! ?e thats wrong
' =
-- stdout
--- expected
+++ actual
@@ --1,3 +-1,4 @@
 thats fine
+thats wrong

ret2libc

LGTM. Just few minor/last nitpicks.

librz/core/cmd/cmd_cmp.c

ret2libc

Good job!

github-actions bot added the RzCore label Oct 18, 2021

XVilka requested changes Oct 19, 2021

View reviewed changes

ret2libc reviewed Oct 19, 2021

View reviewed changes

This was referenced Oct 21, 2021

Create special category for "system" commands #1840

Closed

Port shell commands to new shell #1875

Merged

XVilka mentioned this pull request Oct 25, 2021

Tracking porting the commands parser to the tree-sitter-based one #1342

Open

23 tasks

DMaroo added 10 commits October 26, 2021 09:48

Port c commands to newshell

2a9f521

* `cv`, `cV` and `cg` haven't been ported yet

Add RzCompareOutputMode for specifying mode for rz_core_cmp* API

fbd3276

Use bool return value for __core_cmp_bits

7bd9098

Convert rizin_compare to RZ_API; Modify arguments

43a753a

* Rename `__core_cmp_bits` to `core_cmp_bits`

Remove old shell command handlers for c (cmp) commands

778e1b1

Remove alias handling in cat command handler

61e6e16

Remove c* command

55d811b

Update command description for c commands

94ab497

Add handling for paths relative to home (~) in rz_sys_chdir

381626c

* Also, simplify `cd` command handler

Cleanup rebase merge conflicts

7d26f42

DMaroo force-pushed the c-port branch from 1aac430 to 96e3615 Compare October 26, 2021 04:34

Remove unused handlers for shell commands

aadb9e0

DMaroo force-pushed the c-port branch from 96e3615 to aadb9e0 Compare October 26, 2021 04:39

DMaroo added 2 commits October 28, 2021 17:22

Create API for rz_cmd functions

79f08d8

* Add `rz_cmp.h` and `cmp.c` * Break `cmd_cmp_disasm` into 2 API functions, one for getting the diff, other for printing the diff * Refactor `cmd_cmp.c` to use the new cmd API functions

Merge branch 'dev' into c-port

5f58a1a

DMaroo force-pushed the c-port branch from cca0e75 to 5f58a1a Compare October 28, 2021 12:09

DMaroo added 6 commits October 28, 2021 17:49

Move rz_core_cmpwatcher_* functions from cmd_cmp.c to cmp.c

4274db6

Add compare functions for data and strings

065a914

* Refactor handlers to use the new functions * Add data-data, data-str compare function, and a general print function in the `cmd` API

Add RZ_OUTPUT_MODE_JSON for c

e860516

Use new cmp API for rizin_compare_unified

95ea4c2

Use new cmp API functions in core_cmp_bits

d7efa69

Merge branch 'dev' into c-port

3826092

DMaroo changed the title ~~Port c commands to newshell [WIP]~~ Port c commands to newshell Nov 21, 2021

XVilka assigned ret2libc Nov 22, 2021

ret2libc reviewed Nov 22, 2021

View reviewed changes

librz/core/cmd/cmd_cmp.c Show resolved Hide resolved

ret2libc reviewed Nov 22, 2021

View reviewed changes

librz/core/cmd/cmd_debug.c Outdated Show resolved Hide resolved

librz/core/cmd/cmd_debug.c Outdated Show resolved Hide resolved

librz/core/cmd/cmd_cmp.c Outdated Show resolved Hide resolved

DMaroo force-pushed the c-port branch from a816db8 to 652d74f Compare November 23, 2021 10:45

DMaroo added 2 commits November 23, 2021 19:47

Fix JSON output when using RzCmdStateOutput

c044c61

Fix integraton tests: cmd_cc and cmd_ccc

ab07b3e

DMaroo force-pushed the c-port branch from 652d74f to ab07b3e Compare November 23, 2021 14:17

DMaroo added 2 commits November 23, 2021 19:47

Merge branch 'dev' into c-port

914ab3d

Remove setting value of core->num->value where not necessary

f5e54d0

XVilka approved these changes Nov 24, 2021

View reviewed changes

DMaroo added 4 commits November 24, 2021 14:17

No need for ca and cb to return in $?; Only cx returns in $?

a12f473

Check return value of cx

216ffdd

Free buf to avoid leak

e1f96cb

Merge branch 'dev' into c-port

f898855

DMaroo requested a review from ret2libc November 24, 2021 11:44

DMaroo added 4 commits November 25, 2021 00:17

No need to concatenate arguments manually in cx

7657342

Merge branch 'dev' into c-port

b697af5

Fix cmd_cx integration test

2f53657

Merge branch 'dev' into c-port

22d2212

XVilka added the merge-when-green label Nov 25, 2021

ret2libc reviewed Nov 25, 2021

View reviewed changes

DMaroo added 2 commits November 25, 2021 16:04

Use ternary if condition to make return value setting more concise

be47c76

Merge branch 'dev' into c-port

7a7d865

DMaroo requested a review from ret2libc November 25, 2021 11:02

ret2libc approved these changes Nov 25, 2021

View reviewed changes

DMaroo merged commit 0801f7a into dev Nov 25, 2021

DMaroo deleted the c-port branch November 25, 2021 12:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port `c` commands to newshell #1854

Port `c` commands to newshell #1854

DMaroo commented Oct 18, 2021 •

edited

Loading

XVilka Oct 19, 2021

ret2libc left a comment

XVilka commented Oct 26, 2021

DMaroo commented Oct 26, 2021

ret2libc commented Nov 23, 2021

DMaroo commented Nov 23, 2021

XVilka left a comment

ret2libc left a comment

ret2libc left a comment

Port c commands to newshell #1854

Port c commands to newshell #1854

Conversation

DMaroo commented Oct 18, 2021 • edited Loading

XVilka Oct 19, 2021

Choose a reason for hiding this comment

ret2libc left a comment

Choose a reason for hiding this comment

XVilka commented Oct 26, 2021

DMaroo commented Oct 26, 2021

ret2libc commented Nov 23, 2021

DMaroo commented Nov 23, 2021

XVilka left a comment

Choose a reason for hiding this comment

ret2libc left a comment

Choose a reason for hiding this comment

ret2libc left a comment

Choose a reason for hiding this comment

Port `c` commands to newshell #1854

Port `c` commands to newshell #1854

DMaroo commented Oct 18, 2021 •

edited

Loading