Skip to content

chfn: write the other sub-field only when it is set - #397

Merged
sylvestre merged 4 commits into
mainfrom
fix-362-chfn-other
Nov 8, 2026
Merged

sylvestre merged 4 commits into
mainfrom
fix-362-chfn-other

Conversation

@pierre-warnier

@pierre-warnier pierre-warnier commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #362

chfn always wrote five GECOS sub-fields, so an account with no "other" got a trailing empty field (NewName,,,,). It now writes the four finger fields and adds "other", with its comma, only when "other" is not empty.

Behaviour checked against Ubuntu 26.04's packaged passwd (1:4.17.4-2ubuntu3) in ubuntu:26.04:

GECOS before chfn GECOS after
(empty) -f NewName NewName,,,
A,B,C,D -r R2 A,R2,C,D
A -r R A,R,,
A,B,C,D, -r R A,R,C,D
A,,,, -r R A,R,,
A -f '' ,,,
A,B,C,D,E -o '' A,B,C,D
A,B,C,D,E,F -r R A,R,C,D,E,F
(empty) -o X ,,,,X

Every row is now an integration test (test_chfn_writes_other_only_when_set), which fails on main. The two unit tests that pinned the five-field form are updated, the man page says when "other" is written, and there is a CHANGELOG entry.

Reviews welcome. Without objection, this will be merged after 7 days.

Comment thread tests/by-util/test_chfn.rs Outdated
/// Run `chfn -R` on an account whose GECOS is `gecos` and return the GECOS
/// field written back.
fn gecos_after(gecos: &str, args: &[&str]) -> String {
let dir = tempfile::tempdir().expect("failed to create temp dir");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

could use setup_prefix here, no?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it fits: gecos_after now builds its prefix with setup_prefix, in 11cac62.

Upstream writes the four finger fields and adds "other", with its
comma, only when it is not empty. chfn always wrote five fields, so
every change on an account with no "other" left a trailing empty
sub-field: chfn -f NewName on an empty GECOS gave NewName,,,, where
upstream gives NewName,,,.
gecos_after wrote etc/passwd by hand, which is what setup_prefix does.
@sylvestre
sylvestre merged commit 614293c into main Nov 8, 2026
13 checks passed
@sylvestre

Copy link
Copy Markdown
Collaborator

Thanks for your PR

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.

chfn: writes five GECOS fields, GNU writes four when "other" is empty

2 participants