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

Include values in log messages #24

Merged
merged 9 commits into from
Jan 19, 2024
Merged

Include values in log messages #24

merged 9 commits into from
Jan 19, 2024

Conversation

sgoll
Copy link
Contributor

@sgoll sgoll commented Jan 17, 2024

Description

This makes sure to include values when printing out log messages. This is done by passing the format string with variadic arguments to vsnprintf() to handle all the formatting.

Fixes #22

@sgoll sgoll marked this pull request as draft January 17, 2024 17:34
@sgoll
Copy link
Contributor Author

sgoll commented Jan 17, 2024

FYI, this is being blocked by HMIProject/open62541-sys#9 for proper vsnprintf() support on Microsoft Windows.

@sgoll sgoll marked this pull request as ready for review January 19, 2024 11:58
@sgoll
Copy link
Contributor Author

sgoll commented Jan 19, 2024

You can test the output with RUST_LOG=debug cargo run --example async_client.

Copy link
Collaborator

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

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

By enclosing the invocations of printf() in a loop the code might become more compact and the closure would not be needed. But let's leave it as is. Works and the code is readable.

@uklotzde uklotzde merged commit cb7f370 into main Jan 19, 2024
5 checks passed
@uklotzde uklotzde deleted the log-formatting branch January 19, 2024 12:58
@uklotzde
Copy link
Collaborator

Ouch:

[2024-01-19T13:05:49Z WARN  open62541::client] AcceptAll Certificate Verification. Any remote certificate will be accepted.
[2024-01-19T13:05:49Z INFO  open62541::client] Connecting to endpoint opc.tcp://localhost:4840/OPC_UA_Server
thread 'tokio-runtime-worker' panicked at /home/uk/.cargo/git/checkouts/open62541-bd40fc1209456320/697e248/src/client.rs:274:9:
assertion failed: msg_length + 1 >= msg_buffer.len()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@uklotzde
Copy link
Collaborator

The loop would ensure that the code paths are shared to prevent one-off errors like this.

@sgoll
Copy link
Contributor Author

sgoll commented Jan 19, 2024

@uklotzde Interesting. Can you send me a reproduction? I tested the edge cases and saw no such error on my system. Can the implementations of vsnprintf() differ in what return value they produce?

Cont'd in #29

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.

log_c(): Log messages contain C format strings
2 participants