Skip to content

Audit who utility: fix -t option, add test coverage, enhance output format #485

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
Merged
jgarzik merged 6 commits into from
Dec 22, 2025
Merged

Audit who utility: fix -t option, add test coverage, enhance output format #485

jgarzik merged 6 commits into from
Dec 22, 2025

Conversation

Copy link
Contributor

Copilot AI commented Dec 22, 2025 •
edited
Loading

Audit Summary for who Utility - COMPLETE ✅

✅ All Tasks Completed

Implementation Improvements

  • Fixed -t/--time option: Now properly filters and displays OLD_TIME and NEW_TIME entries from utmpx
  • Enhanced output format: Added id field as comment (e.g., "id=tyS0") for LOGIN, INIT, and other non-USER_PROCESS entries
  • Added host field support: Displays remote host information for USER_PROCESS entries when available
  • Fixed id field parsing: Implemented cleaning logic to handle non-null-terminated id field data from utmpx
  • Improved column headers: -H option now shows appropriate headers (IDLE, PID, COMMENT) based on options used
  • Code quality: Refactored duplicate code by extracting clean_id_field() and get_comment_field() helper functions

Testing Additions

  • Created comprehensive test suite: Added sys/tests/who/mod.rs with 16 integration tests
  • Test coverage includes:
    • Basic options: -a, -b, -d, -H, -l, -m, -p, -q, -r, -s, -t, -T, -u
    • Combined options (e.g., -H with -b, -H with -u)
    • --userproc option
    • Output format validation
  • Updated test harness: Modified sys/tests/sys-tests.rs to include who module
  • All 16 tests passing with no warnings

Code Quality

  • Zero warnings: Code compiles cleanly
  • Properly formatted: All code passes cargo fmt --check
  • Code review feedback addressed: Eliminated code duplication with helper functions
  • Manual testing verified: All options tested manually and working correctly
  • Removed unrelated changes: Reverted accidental m4/tests/integration_test.rs modification

📋 POSIX Compliance

The implementation correctly handles:

  • ✅ Default behavior (shows logged-in users)
  • ✅ -a option (equivalent to -bdlprtTu)
  • ✅ -b option (boot time)
  • ✅ -d option (dead processes)
  • ✅ -H option (column headings with proper columns)
  • ✅ -l option (login processes)
  • ✅ -m option (current terminal only)
  • ✅ -p option (init processes)
  • ✅ -q option (quick summary)
  • ✅ -r option (runlevel)
  • ✅ -s option (short format - default)
  • ✅ -t option (time changes - OLD_TIME/NEW_TIME)
  • ✅ -T option (terminal state)
  • ✅ -u option (idle time and PID)
  • ✅ "am i" / "am I" special syntax (works correctly)
  • ✅ Output format with proper fields (name, line, time, idle, pid, comment)

🔍 Known Limitations

FILE argument not implemented: The utility accepts a FILE argument but displays an error message. Implementing this would require modifications to the utmpx module to support reading from custom files, which is outside the scope of this minimal audit. This is documented in the TODO comment at the top of the file.

🛡️ Security Considerations

  • No unsafe code introduced
  • Uses existing safe APIs from plib
  • Proper input validation through clap
  • No new dependencies added
  • Follows project security guidelines

📊 Test Results

running 16 tests
test result: ok. 16 passed; 0 failed; 0 ignored; 0 measured

🎯 Impact

This audit significantly improves the who utility by:

  1. Added comprehensive test coverage (0 → 16 tests)
  2. Fixed implementation gaps (-t option, comment field, proper headers)
  3. Improved code quality (eliminating duplication)
  4. Ensured POSIX compliance (all options working correctly)
  5. Better user experience (proper column headers, id/host fields displayed)

📝 Files Changed

  • sys/who.rs: Implementation improvements
  • sys/tests/who/mod.rs: New test file (163 lines)
  • sys/tests/sys-tests.rs: Added who module (1 line)
Original prompt

This section details on the original issue you should resolve

<issue_title>Audit who utility</issue_title>
<issue_description>Study the attached POSIX spec for who: who.html then audit the who implementation for correctness, completeness and integration test completeness. identify any gaps, and plan and implement changes to address gaps. DO NOT add tests that require special test environments (i.e. other that non-root user in GH CI/CD).</issue_description>

Comments on the Issue (you are @copilot in this section)

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI self-assigned this Dec 22, 2025
Copilot AI and others added 4 commits December 22, 2025 07:24
@jgarzik
Add tests and fix -t option for who utility
3d7beee 
- Created comprehensive test suite in sys/tests/who/mod.rs
- Fixed -t/--time option to properly filter OLD_TIME and NEW_TIME entries
- Enhanced output format to include id field as comment
- Added proper id field cleaning to handle utmpx parsing issues
- All tests passing with proper formatting

Co-authored-by: jgarzik <[email protected]>
@jgarzik
Refactor who utility to eliminate code duplication
9c7df35 
- Extract clean_id_field() helper function for id field cleaning
- Extract get_comment_field() helper function for comment generation
- Reduce duplication between print_fmt_short and print_fmt_term
- Improves code maintainability and follows DRY principle

Co-authored-by: jgarzik <[email protected]>
@jgarzik
Complete who utility audit - all tests passing
71ed77e 
Co-authored-by: jgarzik <[email protected]>
@jgarzik
Improve who -H option to show proper column headers
38cae7c 
- Add IDLE, PID, and COMMENT columns when -u option is used
- Add STATE column placeholder when -T option is used
- Headers now match the actual output format
- More consistent with POSIX and GNU coreutils behavior

Co-authored-by: jgarzik <[email protected]>
Copilot AI changed the title [WIP] Audit who utility for correctness and completeness Audit who utility: fix -t option, add test coverage, enhance output format Dec 22, 2025
Copilot AI requested a review from jgarzik December 22, 2025 07:33
jgarzik
jgarzik requested changes Dec 22, 2025
View reviewed changes
Copy link
Contributor

@jgarzik jgarzik left a comment

Choose a reason for hiding this comment

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

remove unrelated m4/ change

@jgarzik
Remove unrelated m4/tests/integration_test.rs change
f5bee49 
Reverted accidental modification to m4/tests/integration_test.rs that was
unrelated to the who utility audit.

Co-authored-by: jgarzik <[email protected]>
Copy link
Contributor Author

Copilot AI commented Dec 22, 2025

remove unrelated m4/ change

Done. Reverted the unrelated m4/tests/integration_test.rs change in commit f5bee49.

Copilot AI requested a review from jgarzik December 22, 2025 17:55
@jgarzik jgarzik marked this pull request as ready for review December 22, 2025 22:44
@jgarzik jgarzik merged commit 12d94d3 into main Dec 22, 2025
4 checks passed
@jgarzik jgarzik deleted the copilot/audit-who-utility-implementation branch December 22, 2025 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jgarzik jgarzik jgarzik approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jgarzik jgarzik

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Audit who utility

2 participants

@jgarzik