Skip to content

dircolors: remove unsafe#12228

Open
oech3 wants to merge 1 commit intouutils:mainfrom
oech3:dirc-unsafe-env
Open

dircolors: remove unsafe#12228
oech3 wants to merge 1 commit intouutils:mainfrom
oech3:dirc-unsafe-env

Conversation

@oech3
Copy link
Copy Markdown
Contributor

@oech3 oech3 commented May 11, 2026

No description provided.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 11, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/pr/bounded-memory (was skipped on 'main', now failing)

@oech3 oech3 force-pushed the dirc-unsafe-env branch from 25f4f5b to 50a6eaf Compare May 11, 2026 04:47
@oech3 oech3 marked this pull request as ready for review May 11, 2026 05:07
@xtqqczze
Copy link
Copy Markdown
Contributor

Good work. This is exactly the kind of unsafe worth removing. I think most uses of env::set_var and env::remove_var are unnecessary if we refactor the tests accordingly.

Comment thread src/uu/dircolors/src/dircolors.rs Outdated
@oech3 oech3 force-pushed the dirc-unsafe-env branch from 50a6eaf to 6c7ed3b Compare May 11, 2026 13:17
@oech3
Copy link
Copy Markdown
Contributor Author

oech3 commented May 11, 2026

I think fn run_gnu_cmd can use Command::env to remove unsafes.

@xtqqczze
Copy link
Copy Markdown
Contributor

Yeah, we should definitely do that instead of mutating the environment. Regardless of the unsafe aspect, mutating global environment state makes the code harder to reason about and can interfere with optimizations.

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