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.

Sign up for GitHub

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

Jump to bottom

Use execve instead of std::system #970

Closed
ken-matsui opened this issue Aug 12, 2024 · 4 comments · Fixed by #986
Closed

Use execve instead of std::system #970

ken-matsui opened this issue Aug 12, 2024 · 4 comments · Fixed by #986
Labels
good first issue Good for newcomers

Comments

@ken-matsui
Copy link
Member

https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152177

We may want to implement a Command class to hide this complexity, like Rust's std::process::Command.

The only location we use std::system is currently the following (instead of taking std::string_view, let's take the Command class):

poac/src/Algos.cc

Line 44 in 7c4f62e

const int status = std::system(cmd.data());

After addressing this, we can enable cert-env33-c in clang-tidy:

poac/.clang-tidy

Line 14 in 7c4f62e

-cert-env33-c

@ken-matsui ken-matsui added the good first issue Good for newcomers label Aug 12, 2024
zimirza added a commit to zimirza/poac that referenced this issue Sep 12, 2024
@zimirza
refactor: cmd
9f33792 
closes poac-dev#970
@zimirza
Copy link

zimirza commented Sep 12, 2024

I hope it is alright if I started to implement an abstraction for std::system (not yet complete). This is how it could be used: Command("cd").arg("poac").output()

@ken-matsui
Copy link
Member Author

That's great to hear, thank you for working on this!

@ken-matsui
Copy link
Member Author

However, we would like to avoid using malloc family directly. Instead, let's use higher-level libraries, such as std::string.

@zimirza
Copy link

zimirza commented Sep 13, 2024

Sure, I have refactored it to use std::string. I can create a draft pull request.

@zimirza zimirza mentioned this issue Sep 13, 2024
Closed
@yaito3014 yaito3014 mentioned this issue Sep 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Refactor command execution yaito3014/poac
Use execve instead of std::system zimirza/poac
2 participants
@ken-matsui @zimirza