Skip to content

Logging fixups and cleanups#555

Merged
jnovy merged 5 commits intocontainers:mainfrom
kolyshkin:log-fixups
Apr 24, 2025
Merged

Logging fixups and cleanups#555
jnovy merged 5 commits intocontainers:mainfrom
kolyshkin:log-fixups

Conversation

@kolyshkin
Copy link
Collaborator

I was looking into how conmon logs things and found out a few things that can be improved, so here's this.

Please review commit by commit; see individual commits for details.

The ndebugf macro already adds \n so we don't have to.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Use nwarn instead of pwarn since we're not interested in errno here.

2. Add some context to the message.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
And convert 2 of its users to use nwarnf with %m.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Copy link
Collaborator

@jnovy jnovy left a comment

Choose a reason for hiding this comment

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

Thanks for this, just one comment related to malloc() + errno.

In here, error and errorf print strerror(errno), but the only case
errno is set is when malloc() fails (and the errno is invariably
ENOMEM so it doesn't make much sense to print it anyway).

1. Remove adding strerror(errno) from these macros.

2. Redefine error via errorf to simplify.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
While %m is GNU extension, it is also supported by alternative libc
versions (uclibc, musl) as well as by FreeBSD 12+.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@jnovy jnovy merged commit 6f5e1d2 into containers:main Apr 24, 2025
28 of 29 checks passed
@jnovy
Copy link
Collaborator

jnovy commented Apr 24, 2025

All good now, thanks!

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