Skip to content

who: Add who command #25943

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.

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

ebanner
Copy link
Contributor

@ebanner ebanner commented May 20, 2025

image

@ebanner ebanner marked this pull request as draft May 20, 2025 21:00
@github-actions github-actions bot added 👀 pr-needs-review PR needs review from a maintainer or community member and removed 👀 pr-needs-review PR needs review from a maintainer or community member labels May 20, 2025
@nico nico added the ⏳ pr-waiting-for-author PR is blocked by feedback / code changes from the author label May 20, 2025
@ebanner ebanner force-pushed the add-who-command branch from f47f8a2 to 6bfe217 Compare May 21, 2025 15:50
@github-actions github-actions bot removed the ⏳ pr-waiting-for-author PR is blocked by feedback / code changes from the author label May 21, 2025
@ebanner ebanner force-pushed the add-who-command branch 3 times, most recently from 1aca307 to 72d8ca1 Compare May 21, 2025 16:00
@ebanner ebanner force-pushed the add-who-command branch from 72d8ca1 to 4969fae Compare May 21, 2025 17:40
Copy link
Collaborator

@kleinesfilmroellchen kleinesfilmroellchen left a comment

Choose a reason for hiding this comment

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

In need of C-ism removal, but functionality is good :^)

{
TRY(Core::System::pledge("stdio rpath"));
TRY(Core::System::unveil("/var/run/utmp", "r"));
TRY(Core::System::unveil("/etc", "r"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems like the program only ever needs to read /etc/passwd here, please be more specific with your unveil then.

Comment on lines +33 to +36
auto time = static_cast<time_t>(login_at);
auto* tm = localtime(&time);
char timebuf[32];
strftime(timebuf, sizeof(timebuf), "%b %d %H:%M", tm);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Even though it’s a bad interface and I have a branch somewhere that fixes it, please rather use Core::LocalTime. We’re not C.

json.as_object().for_each_member([&](auto& key, auto& value) {
auto const& obj = value.as_object();
auto uid = obj.get_i32("uid"sv).value();
auto* passwd = getpwuid(uid);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am pretty sure Core::Account can do this with better error handling.

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.

5 participants