Change -, --, = argument and = command meaning #930

seanachao · 2021-03-29T10:55:16Z

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 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

Remove the = special flag from Rizin's startup flags to avoid possible confusion with the client/server = meaning in interactive mode
Move the = interactive client/server family command to R (new) interactive family command (short for "Remote")
-- is used to signal "end of flags" to be consistent with the standard command line utilities syntax.
...

Test plan

From #615:

rizin
rizin --
rizin -test
rizin -- -test
rizin -d -- -test

From #616:

rizin =
rizin - < test/bins/elf/ls
for the test

➜  rizin_bak git:(startissue) rizin 
 -- The '?' command can be used to evaluate math expressions. Like this: '? (0x34+22)*4'
[0x00000000]> 

➜  rizin_bak git:(startissue) rizin --
 -- Seek at relative offsets with 's +<offset>' or 's -<offset>'
[0x00000000]> 

➜  rizin_bak git:(startissue) ✗ rizin -- -test
 -- Review all the subcommands of aa to see better ways to analyze your targets.
[0x00000000]> 

➜  rizin_bak git:(startissue) ✗ rizin -d -- -test
Process with PID 1981196 started...
= attach 1981196 1981196
bin.baddr 0x555555554000
Using 0x555555554000
asm.bits 64
 -- Find wide-char strings with the '/w <string>' command
[0x7ffff7fd3090]> 

➜  rizin_bak git:(startissue) rizin =
 -- Use scr.accel to browse the file faster!
[0x00000000]> 

➜  rizin_bak git:(startissue) ✗ rizin - < test/bins/elf/ls
^D
 -- Get a free shell with 'rz_gg -i exec -x'
[0x00000000]>

Closing issues

Fix #616 Swap - (single dash) and = (equal sign) meaning
Fix #615 -- (double dash) should be treated as "end of flags" | Allow launching Rizin without a

...

XVilka · 2021-03-29T10:57:08Z

Please don't open a new PR every time!
Please rebase on top of the latest dev branch to resolve the conflicts.

Previos PRs are:

seanachao · 2021-03-29T11:04:08Z

2. Please rebase on top of the latest dev branch to resolve the conflicts.

Please don't open a new PR every time!

Please rebase on top of the latest dev branch to resolve the conflicts.

Previos PRs are:

Startissue #921

Startissue #922

Startissue #927

So Sorry, and I know .

fangxlmr · 2021-03-29T11:04:42Z

@seanachao And also, fixup or squash in rebasing on top of dev will help a lot on clear commit logs.

fangxlmr · 2021-03-29T11:06:11Z

@seanachao Why do you close this PR again? O_O

seanachao · 2021-03-29T11:26:52Z

@seanachao Why do you close this PR again? O_O

I think this pr is not right, so I close it again,so sorry. If I want to commit agin, what should I do?

seanachao · 2021-03-29T11:31:52Z

@seanachao Why do you close this PR again? O_O

XVilka · 2021-03-29T11:39:13Z

Just force-push to the branch: git push -f <remote> <branch>

seanachao · 2021-03-29T11:44:11Z

Just force-push to the branch: git push -f <remote> <branch>

Okay, thanks!

karliss · 2021-03-29T15:01:47Z

@seanachao @wargio Please try to give PRs a descriptive tittles so that developers looking at the PR list can get basic understanding of what kind of changes are inside. "Fix #nnnn" without any additional information isn't very useful either since I doubt anyone can remember all the issue numbers and issue numbers in PR tittle are not automatically converted to links.

seanachao · 2021-03-29T15:47:59Z

@seanachao @wargio Please try to give PRs a descriptive tittles so that developers looking at the PR list can get basic understanding of what kind of changes are inside. "Fix #nnnn" without any additional information isn't very useful either since I doubt anyone can remember all the issue numbers and issue numbers in PR tittle are not automatically converted to links.

@karliss okey, I added the title of the issue.

ret2libc

Check for = in cmd_descs.yaml.

librz/core/cmd_descs/cmd_remote.yaml

librz/main/rizin.c

librz/util/getopt.c

ret2libc · 2021-03-29T16:15:22Z

I've added to the comment0 some things to try as tests, mentioned in the referenced issues. I'm not sure yet if automatic tests were added for those, but they should be tested manually before approving this.

fangxlmr · 2021-03-30T01:21:01Z

And I don't think this is a good git commit here. You probably messed up something locally (e.g. run git pull without -r flag) because this is not what rebase does. You might wanna rephrase those commits and make them clear first.

wargio

LGTM.

XVilka

Please wait until @ret2libc take a look.

ret2libc

in rizin -h: -= perform !=! command to run all commands remotely this should probably be R=!
./build-change-args/binrz/rizin/rizin - -c "ie" should this work or give out an error? not sure
in rizin -h: -C file is host:port (alias for -c+=http://%s/cmd/) should probably be -cR+ .....
grep for =!= and !=! and replace them as appropriate (also in scanner.c)
grep for =! and replace it with R! as appropriate (e.g. in librz/io/p/io_rap.c and others)
grep for =h and replace with Rh as appropriate (e.g. in librz/core/rtr_http.c)
| Rh port listen for http connections (rizin -qc=H /bin/ls) should probably be cRH /bin/ls
| R&r <port> # Start rap server in background (same as '&_=h') not sure where that &_=h came from, but it seems definitely wrong
not only related to your PR, but could you fix librz/core/rtr.c:rz_core_rtr_cmd() because it still looks for =: and =&: instead of Rr and R&r/Rh&.

seanachao · 2021-04-13T06:02:50Z

in rizin -h: -= perform !=! command to run all commands remotely this should probably be R=!

./build-change-args/binrz/rizin/rizin - -c "ie" should this work or give out an error? not sure

in rizin -h: -C file is host:port (alias for -c+=http://%s/cmd/) should probably be -cR+ .....

grep for =!= and !=! and replace them as appropriate (also in scanner.c)

grep for =! and replace it with R! as appropriate (e.g. in librz/io/p/io_rap.c and others)

grep for =h and replace with Rh as appropriate (e.g. in librz/core/rtr_http.c)

| Rh port listen for http connections (rizin -qc=H /bin/ls) should probably be cRH /bin/ls

| R&r <port> # Start rap server in background (same as '&_=h') not sure where that &_=h came from, but it seems definitely wrong

not only related to your PR, but could you fix librz/core/rtr.c:rz_core_rtr_cmd() because it still looks for =: and =&: instead of Rr and R&r/Rh&.

@ret2libc thanks for your comments, And I have some doubts.

| R&r <port> # Start rap server in background (same as '&_=h') not sure where that &_=h came from, but it seems definitely wrong . It is In dev branch , there is same as &_=h in line 28 , how should I fix it?
./build-change-args/binrz/rizin/rizin - -c "ie" should this work or give out an error? not sure . After swap = and - meaning, ./build/binrz/rizin/rizin = -c "ie" also have no output. How should I fix it, dou you have some advice ?

ret2libc · 2021-04-14T08:23:40Z

@ret2libc thanks for your comments, And I have some doubts.
* `| R&r <port>              # Start rap server in background (same as '&_=h')` not sure where that `&_=h` came from, but it seems definitely wrong .  
It is In dev branch , there is same as &_=h in line 28 , how should I fix it?

I did some changes in the past to these commands names as well, so probably there were a series of changes and somehow that string was forgotten and partially changed.

I think this should be & Rr

./build-change-args/binrz/rizin/rizin - -c "ie" should this work or give out an error? not sure .

After swap = and - meaning, ./build/binrz/rizin/rizin = -c "ie" also have no output. How should I fix it, dou you have some advice ?

@XVilka @caribpa do you have any advice here? I would expect invalid combinations of argument flags to fail early and hard, instead of doing "something".

XVilka · 2021-04-14T08:42:19Z

@XVilka @caribpa do you have any advice here? I would expect invalid combinations of argument flags to fail early and hard, instead of doing "something".

Absolutely agree, it should be an error message. 👍

@seanachao please also rebase against the latest dev.

XVilka · 2021-04-16T09:55:11Z

Seems like it broke tree-sitter parser cc @ret2libc

1 failure:

expected / actual

  1. R commands:
    (commands (arged_command (cmd_identifier)) (arged_command (cmd_identifier) (args (arg (arg_identifier)))) (cmd_identifier)) (arged_command (cmd_identifier) (args (arg (arg_identifier)) (arg (arg_identifier)))) (arged_command (cmd_identifier) (args (arg (arg_identifier)))) (arged_command (cmd_identifier) (args (arg (arg_identifier)))) (arged_command (cmd_identifier) (args (arg (arg_identifier)))) (arged_command (cmd_identifier) (args (arg (arg_identifier)))) (arged_command (cmd_identifier)) (arged_command (cmd_identifier)) (arged_command (cmd_identifier) (args (arg (arg_identifier)) (arg (arg_identifier)))) (arged_command (cmd_identifier) (args (arg (arg_identifier))))) 
Error: Process completed with exit code 1.

ret2libc · 2021-04-16T13:29:21Z

Seems like it broke tree-sitter parser cc @ret2libc

1 failure:

expected / actual

  1. R commands:
    (commands (arged_command (cmd_identifier)) (arged_command (cmd_identifier) (args (arg (arg_identifier)))) (cmd_identifier)) (arged_command (cmd_identifier) (args (arg (arg_identifier)) (arg (arg_identifier)))) (arged_command (cmd_identifier) (args (arg (arg_identifier)))) (arged_command (cmd_identifier) (args (arg (arg_identifier)))) (arged_command (cmd_identifier) (args (arg (arg_identifier)))) (arged_command (cmd_identifier) (args (arg (arg_identifier)))) (arged_command (cmd_identifier)) (arged_command (cmd_identifier)) (arged_command (cmd_identifier) (args (arg (arg_identifier)) (arg (arg_identifier)))) (arged_command (cmd_identifier) (args (arg (arg_identifier))))) 
Error: Process completed with exit code 1.

@seanachao in scanner.c there is:

static bool is_equal_cmd(const char *s) {
	return s[0] == '=';
}

static bool is_mid_command(const char *res, int len, const int32_t ch) {
	.....
	return iswalnum (ch) || ch == '$' || ch == '?' || ch == '.' || ch == '!' ||
		ch == ':' || ch == '+' || ch == '=' || ch == '/' || ch == '*' ||
		ch == '-' || ch == ',' || ch == '&' || ch == '_' ||
		(is_interpret_cmd (res) && ch == '(') ||
		(is_equal_cmd (res) && ch == '<') || (is_at_cmd (res) && ch == '@');
}

Please rename is_equal_cmd to is_remote_cmd and s[0] == '=' to s[0] == 'R'. It should fix the problem. If not, let me know!

ret2libc · 2021-04-19T08:06:47Z

@seanachao

$ ./build-change-args/binrz/rizin/rizin -
-: invalid combinations of argument flags - (null)

wargio · 2021-04-19T08:09:47Z

@seanachao

$ ./build-change-args/binrz/rizin/rizin -
-: invalid combinations of argument flags - (null)

shouldn't it now be rizin = ?

ret2libc · 2021-04-19T08:13:48Z

@seanachao

$ ./build-change-args/binrz/rizin/rizin -
-: invalid combinations of argument flags - (null)

shouldn't it now be rizin = ?

rizin = opens a malloc://1024 file, rizin - gets the input binary from stdin.

@seanachao also:

[0x00006b60]> Rr 33333
Error: Unknown host

Before it was working.

By the way, let me tell you that this is really a great job and sorry for the many different changes you are requested, but the more things you modify, the more things need to be checked. Anyway, I think you are missing just few details! Thanks again for all this!

XVilka · 2021-04-19T10:08:40Z

Is there a test for this situation specifically?

$ touch -- -test
$ ls -- -test
# -test
$ echo hi | tee -- -test
# hi
$ rizin -- -test

I scrolled the changes and haven't seen a new test for this in test/db/, might have skipped due to the huge amount of changes.

seanachao · 2021-04-19T13:43:33Z

Is there a test for this situation specifically?
$ touch -- -test
$ ls -- -test
# -test
$ echo hi | tee -- -test
# hi
$ rizin -- -test
I scrolled the changes and haven't seen a new test for this in test/db/, might have skipped due to the huge amount of changes.

@XVilka @ret2libc, the test I haven't add , when my test json such as this

NAME=--
FILE=-test
ARGS=--
CMDS=<<EOF
e cfg.bigendian=true
pxw 16
EOF
EXPECT=<<EOF
0x68690aff 0xffffffff 0xffffffff 0xffffffff  hi..............
EOF
RUN

the test cmd is

RZ_NOPLUGINS=1 rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -N -- -Qc 'e cfg.bigendian=true
pxw 16
' -test

how could I modify the test json to get the test cmd like

RZ_NOPLUGINS=1 rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -N  -Qc 'e cfg.bigendian=true
pxw 16
'  -- -test

seanachao · 2021-04-19T13:53:31Z

@seanachao
$ ./build-change-args/binrz/rizin/rizin -
-: invalid combinations of argument flags - (null)
shouldn't it now be rizin = ?
rizin = opens a malloc://1024 file, rizin - gets the input binary from stdin.

@seanachao also:
[0x00006b60]> Rr 33333
Error: Unknown host
Before it was working.

By the way, let me tell you that this is really a great job and sorry for the many different changes you are requested, but the more things you modify, the more things need to be checked. Anyway, I think you are missing just few details! Thanks again for all this!

@ret2libc , For Rr wasn't woring, when I try to resolve the question

not only related to your PR, but could you fix librz/core/rtr.c:rz_core_rtr_cmd() because it still looks for =: and =&: instead of Rr and R&r/Rh&

I use 'r' to replace ':' ,so Rr wasn't working , do you have any advice to fix rz_core_rtr_cmd()

	// "=:"
	- if (*input == ':' && !strchr(input + 1, ':')) {
	+ if (*input == 'r' && !strchr(input + 1, 'r')) {

ret2libc · 2021-04-20T08:59:18Z

@XVilka @ret2libc, the test I haven't add , when my test json such as this
NAME=--
FILE=-test
ARGS=--
CMDS=<<EOF
e cfg.bigendian=true
pxw 16
EOF
EXPECT=<<EOF
0x68690aff 0xffffffff 0xffffffff 0xffffffff  hi..............
EOF
RUN

Good point. I think it is not possible right now... @thestr4ng3r is it?

ret2libc · 2021-04-20T09:09:02Z

@ret2libc , For Rr wasn't woring, when I try to resolve the question

Ok..... I realize just now things are really messy and probably it's better to just rewrite most of those things with newshell in mimd, in the future. For now, just revert that change with r/: in rz_core_rtr_cmd. Sorry for that.

ret2libc

Ok, good work!! Thanks a lot for all the changes. I think it's ready now. I've tried it a bit and it seems to work fine. I think there is still some adjustments to do on the remote part, but we can probably do it in a separate PR that will be easier to review/smaller.

Thanks again!

seanachao requested review from kazarmy, ret2libc, thestr4ng3r, wargio and XVilka as code owners March 29, 2021 10:55

github-actions bot added command GDB rizin RzCore RzIO labels Mar 29, 2021

seanachao closed this Mar 29, 2021

seanachao reopened this Mar 29, 2021

seanachao force-pushed the startissue branch from ce73403 to 808bfbb Compare March 29, 2021 11:43

This comment has been minimized.

Sign in to view

wargio changed the title ~~Startissue~~ Fix #616 #615 Mar 29, 2021

wargio linked an issue Mar 29, 2021 that may be closed by this pull request

Swap - (single dash) and = (equal sign) meaning #616

Closed

karliss changed the title ~~Fix #616 #615~~ Change -, --, = argument and = command meaning Mar 29, 2021

ret2libc reviewed Mar 29, 2021

View reviewed changes

wargio approved these changes Apr 8, 2021

View reviewed changes

XVilka approved these changes Apr 9, 2021

View reviewed changes

ret2libc reviewed Apr 12, 2021

View reviewed changes

seanachao force-pushed the startissue branch 2 times, most recently from f251bea to a916da8 Compare April 16, 2021 09:51

seanachao force-pushed the startissue branch from a916da8 to 9fde6a9 Compare April 16, 2021 10:47

seanachao force-pushed the startissue branch 4 times, most recently from e6f98c6 to c8b385a Compare April 18, 2021 01:49

Swap = & - R (rizinorg#615)&(rizinorg#616)

7eec165

seanachao force-pushed the startissue branch from c8b385a to 9552879 Compare April 20, 2021 14:13

Fix R replace =

1ed4882

seanachao force-pushed the startissue branch from 9552879 to 1ed4882 Compare April 20, 2021 14:16

ret2libc approved these changes Apr 21, 2021

View reviewed changes

XVilka merged commit a2f8631 into rizinorg:dev Apr 21, 2021

ret2libc mentioned this pull request May 19, 2021

File is still required when passing one option to rizin #1150

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change -, --, = argument and = command meaning #930

Change -, --, = argument and = command meaning #930

seanachao commented Mar 29, 2021 •

edited

Loading

XVilka commented Mar 29, 2021 •

edited

Loading

seanachao commented Mar 29, 2021

fangxlmr commented Mar 29, 2021

fangxlmr commented Mar 29, 2021 •

edited

Loading

seanachao commented Mar 29, 2021

seanachao commented Mar 29, 2021

XVilka commented Mar 29, 2021

seanachao commented Mar 29, 2021

This comment has been minimized.

karliss commented Mar 29, 2021 •

edited

Loading

seanachao commented Mar 29, 2021

ret2libc left a comment

ret2libc commented Mar 29, 2021

fangxlmr commented Mar 30, 2021

wargio left a comment

XVilka left a comment

ret2libc left a comment

seanachao commented Apr 13, 2021 •

edited

Loading

ret2libc commented Apr 14, 2021

XVilka commented Apr 14, 2021

XVilka commented Apr 16, 2021

ret2libc commented Apr 16, 2021

ret2libc commented Apr 19, 2021

wargio commented Apr 19, 2021

ret2libc commented Apr 19, 2021

XVilka commented Apr 19, 2021

seanachao commented Apr 19, 2021 •

edited

Loading

seanachao commented Apr 19, 2021

ret2libc commented Apr 20, 2021

ret2libc commented Apr 20, 2021

ret2libc left a comment

Change -, --, = argument and = command meaning #930

Change -, --, = argument and = command meaning #930

Conversation

seanachao commented Mar 29, 2021 • edited Loading

XVilka commented Mar 29, 2021 • edited Loading

seanachao commented Mar 29, 2021

fangxlmr commented Mar 29, 2021

fangxlmr commented Mar 29, 2021 • edited Loading

seanachao commented Mar 29, 2021

seanachao commented Mar 29, 2021

XVilka commented Mar 29, 2021

seanachao commented Mar 29, 2021

This comment has been minimized.

karliss commented Mar 29, 2021 • edited Loading

seanachao commented Mar 29, 2021

ret2libc left a comment

Choose a reason for hiding this comment

ret2libc commented Mar 29, 2021

fangxlmr commented Mar 30, 2021

wargio left a comment

Choose a reason for hiding this comment

XVilka left a comment

Choose a reason for hiding this comment

ret2libc left a comment

Choose a reason for hiding this comment

seanachao commented Apr 13, 2021 • edited Loading

ret2libc commented Apr 14, 2021

XVilka commented Apr 14, 2021

XVilka commented Apr 16, 2021

ret2libc commented Apr 16, 2021

ret2libc commented Apr 19, 2021

wargio commented Apr 19, 2021

ret2libc commented Apr 19, 2021

XVilka commented Apr 19, 2021

seanachao commented Apr 19, 2021 • edited Loading

seanachao commented Apr 19, 2021

ret2libc commented Apr 20, 2021

ret2libc commented Apr 20, 2021

ret2libc left a comment

Choose a reason for hiding this comment

seanachao commented Mar 29, 2021 •

edited

Loading

XVilka commented Mar 29, 2021 •

edited

Loading

fangxlmr commented Mar 29, 2021 •

edited

Loading

karliss commented Mar 29, 2021 •

edited

Loading

seanachao commented Apr 13, 2021 •

edited

Loading

seanachao commented Apr 19, 2021 •

edited

Loading