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.

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

Add interactive mode to r2r #16466

Merged
merged 7 commits into from
Apr 8, 2020
Merged

Add interactive mode to r2r #16466

merged 7 commits into from
Apr 8, 2020

Conversation

thestr4ng3r
Copy link
Contributor

@thestr4ng3r thestr4ng3r commented Apr 7, 2020

The final nail in r2r.js' coffin.

Fixes #15662

@@ -387,6 +397,19 @@ int main(int argc, char **argv) {
}
r_pvector_clear (&workers);

ut64 seconds = (r_sys_now () - time_start) / 1000000;
Copy link
Collaborator

Choose a reason for hiding this comment

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

use the constants!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you wrote that code in the first place :-P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should do that change in another pr, where we move some stuff to r_util.

@radareorg radareorg deleted a comment from thestr4ng3r Apr 7, 2020
Copy link
Collaborator

@radare radare left a comment

Choose a reason for hiding this comment

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

yay

@thestr4ng3r thestr4ng3r requested a review from radare April 7, 2020 21:46
@thestr4ng3r thestr4ng3r marked this pull request as ready for review April 7, 2020 21:46
@XVilka XVilka added the r2r Regression tests label Apr 8, 2020
binr/r2r/r2r.c Outdated
if (!test->cmds.value) {
return;
}
char *name;
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe worth = NULL; in theory when returning -1 name will not be initialized, so the free below it will crash

binr/r2r/r2r.c Outdated
eprintf ("Failed to open tmp file\n");
return;
}
write (fd, test->cmds.value, strlen (test->cmds.value));
Copy link
Collaborator

Choose a reason for hiding this comment

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

(void) the return on purpose or check the return value and report proper error in case it cant write

binr/r2r/r2r.c Outdated
}
size_t len = strlen (trimmed);
// strip all trailing newlines
while (len && trimmed[len - 1] == '\n') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

that wont work on CRLF files, also we have r_str apis for that

binr/r2r/r2r.c Outdated
// if it's multiline we want exactly one trailing newline
if (strchr (trimmed, '\n')) {
char *tmp = newcmds;
newcmds = r_str_newf ("%s\n", trimmed);
Copy link
Collaborator

Choose a reason for hiding this comment

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

newcmds can be null and then strchr will crash

Copy link
Collaborator

@radare radare left a comment

Choose a reason for hiding this comment

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

manpage not updated, neither the oneline help message

@radare radare added this to the 4.4.0 - pangolin milestone Apr 8, 2020
@radare
Copy link
Collaborator

radare commented Apr 8, 2020

there are cornflakes in r2r.c now pls fix too

@thestr4ng3r
Copy link
Contributor Author

Cornflakes are eaten and comets are addressed.

@thestr4ng3r thestr4ng3r requested a review from radare April 8, 2020 15:32
Copy link
Collaborator

@radare radare left a comment

Choose a reason for hiding this comment

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

now lgtm

@radare radare merged commit cda24be into master Apr 8, 2020
@thestr4ng3r thestr4ng3r deleted the r2r-i branch April 8, 2020 16:52
Emi1305 pushed a commit to Emi1305/radare2 that referenced this pull request Jul 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
r2r Regression tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Autofix feature for C suite tests
3 participants