Skip to content

Timestamp fix#543

Merged
haircommander merged 2 commits intocontainers:mainfrom
jnovy:timestamp-fix
Feb 11, 2025
Merged

Timestamp fix#543
haircommander merged 2 commits intocontainers:mainfrom
jnovy:timestamp-fix

Conversation

@jnovy
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

@jnovy
Copy link
Copy Markdown
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.

@jnovy jnovy requested a review from saschagrunert January 23, 2025 14:39
@jnovy jnovy requested a review from haircommander January 31, 2025 10:45
@haircommander
Copy link
Copy Markdown
Collaborator

can you run make fmt please?

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
Copy Markdown
Collaborator Author

jnovy commented Feb 11, 2025

Done @haircommander

@haircommander haircommander merged commit 41e2c0d into containers:main Feb 11, 2025
25 of 29 checks passed
@jnovy jnovy deleted the timestamp-fix branch April 16, 2025 06:46
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