8000 xargs doesn't limit command line lengths properly · Issue #164 · uutils/findutils · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

xargs doesn't limit command line lengths properly #164

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 “ 8000 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
Open
tavianator opened this issue May 25, 2022 · 5 comments
Open

xargs doesn't limit command line lengths properly #164

tavianator opened this issue May 25, 2022 · 5 comments

Comments

@tavianator
Copy link
Contributor 
$ find ~ -print0 | ./target/debug/xargs -0 echo >/dev/null
Error: Command could not be run: Argument list too long (os error 7)

From a quick look, there's at least an issue here:

findutils/src/xargs/mod.rs

Lines 141 to 146 in 36e3229

#[cfg(unix)]
fn count_osstr_chars_for_exec(s: &OsStr) -> usize {
use std::os::unix::ffi::OsStrExt;
// Include +1 for the null terminator.
s.as_bytes().len() + 1
}

This needs to count not only the length of the string, but also the size of the pointer that ends up in argv/envp.

It might be worth using the https://crates.io/crates/argmax crate to do this accounting for us. It could also help for #6.
@Every2
Copy link
Contributor
Every2 commented Apr 14, 2025 •
edited
Loading

Hi, @tavianator . What you think about use something like:

#[cfg(unix)]
fn count_osstr_chars_for_exec(s: &OsStr) -> (usize, usize) {
    use std::os::unix::ffi::OsStrExt;
    let cmd = Command::new(s);
    let args: Vec<&OsStr> = cmd.get_args().collect();
    // Include +1 for the null terminator.
    (s.as_bytes().len() + 1, args.len())
}

To this issue? We mantain the count for string and get argv count together.

Reference

@tavianator
Copy link
Contributor Author

@Every2 I would strongly recommend using https://crates.io/crates/argmax, it already contains a well-tested implementation of command line length limiting. It was also recently added to this project's find implementation: #515

@Every2
Copy link
Contributor
Every2 commented Apr 16, 2025

@tavianator I see. Just confirming if I understood correctly. The structure of the code shouldn't be changed, only the function count_osstr_chars_for_exec instead of using an OsStr, use argmax equivalent (I believe argmax::command and it's methods)?

@tavianator
Copy link
Contributor Author

The structure of the code shouldn't be changed, only the function count_osstr_chars_for_exec

No, i feel like the code in xargs/mod.rs should be mostly re-written to take advantage of the argmax crate instead of using its own logic (MaxArgsCommandSizeLimiter etc.)

@Every2
Copy link
Contributor
Every2 commented Apr 17, 2025

No, i feel like the code in xargs/mod.rs should be mostly re-written to take advantage of the argmax crate instead of using its own logic (MaxArgsCommandSizeLimiter etc.)

I'll start working on it then! One more thing. Is it better to send small PRs or send it all at once?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@tavianator @Every2
0