Skip to content

Conversation

@jiangenj
Copy link
Contributor

@jiangenj jiangenj commented Apr 7, 2025


Before sending a pull request, please review Contribution Guidelines:
https://github.com/google/syzkaller/blob/master/docs/contributing.md


@a-nogikh
Copy link
Collaborator

a-nogikh commented Apr 7, 2025

Hi Joey!
This PR completely changes the semantics of the RepairScript parameter - it used to be the name of the local file containing the script, now it's the script body itself / the name of the file on the device. Is that change really necessary?

The second change (vm/adb: pass device|console info to script) looks good to me.

@jiangenj
Copy link
Contributor Author

jiangenj commented Apr 8, 2025

it used to be the name of the local file containing the script, now it's the script body itself / the name of the file on the

Hi @a-nogikh , RepairScript accepts script file only, even with the change, it is still using the script filename only.

The difference between sh -c content and sh script can sometimes cause confusion due to how they handle commands and scripts.

sh -c content: This command runs the string content as a shell command. If content contains multiple commands or special characters, it might need proper quoting or escaping. For example, sh -c "echo Hello; echo World" works, but sh -c echo Hello; echo World will not because the shell interprets echo Hello; as a separate command.

sh script: This command runs the script file named script. The script file can contain multiple lines of commands, and the shell will execute them sequentially. This method is generally more straightforward as it doesn't require special handling of quotes or escaping within the script.

@a-nogikh
Copy link
Collaborator

a-nogikh commented Apr 8, 2025

Ah, I see, the script is anyway executed on the host side. Thanks for the clarification.

@a-nogikh a-nogikh added this pull request to the merge queue Apr 8, 2025
Merged via the queue into google:master with commit 6e02d8c Apr 8, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants