Skip to content
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

Timestamp fix #543

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Timestamp fix #543

wants to merge 2 commits into from

Conversation

jnovy
Copy link
Collaborator

@jnovy jnovy commented Jan 15, 2025

Make timestamp generation never fail.

If, for whatever reason, quering current time fails then the whole log message is dropped.

This PR assures the log message is not lost and the output is done with the default timestamp.

Changing log permissions to 0640 would allow the administrator to
set sticky group on the log directory, and for a selected
log-users (in a specific group) without root-permissions to
read the log files.

Fixes containers#539

Signed-off-by: Jindrich Novy <jnovy@redhat.com>
@jnovy
Copy link
Collaborator Author

jnovy commented Jan 15, 2025

@haircommander PTAL

struct timespec ts;
/* Initialize timestamp variables with sensible defaults. */
struct timespec ts = {.tv_sec = 0, .tv_nsec = 0};
struct tm current_tm = {.tm_year = 70, .tm_mon = 0, .tm_mday = 1, .tm_hour = 0, .tm_min = 0, .tm_sec = 0, .tm_gmtoff = 0};
Copy link
Collaborator

Choose a reason for hiding this comment

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

if we're setting all of these when we fail anyway, do we need to initialize? we could leave this struct timespec ts; and then set the default if it fails

If, for whatever reason, quering current time fails
then the whole log message is dropped.

This PR assures the log message is not lost and
the output is done with the default timestamp.

Signed-off-by: Jindrich Novy <jnovy@redhat.com>
@jnovy
Copy link
Collaborator Author

jnovy commented Jan 16, 2025

Fair enough - there's no need to set it twice - just left the zero initialization @haircommander PTAL again.

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