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

Write to stderr in ParseAPI::parsing_printf #1650

Merged
merged 6 commits into from Dec 27, 2023

Conversation

hainest
Copy link
Contributor

@hainest hainest commented Dec 24, 2023

Using a per-thread file here is cumbersome. Including the thread id in the message and synchronizing vprintf makes this work for multiple threads.

Using a per-thread file here is cumbersome. Including the thread id in
the message and synchronizing vprintf makes this work for multiple
threads.
@hainest hainest added code cleanup Bring the code up to modern standards or remove deprecated features parseAPI This issue is directly related to parseAPI labels Dec 24, 2023
@hainest hainest requested a review from kupsch December 24, 2023 17:56
@hainest hainest self-assigned this Dec 24, 2023
This is a contentious warning that was added to clang-15. See
(https://discourse.llvm.org/t/rfc-c-buffer-hardening/65734/85) for
details. Disable it for now.

This should be converted to a standard diagnostic suppression in
common/compiler_diagnostics.h.
That was an inadvertent change.
parseAPI/src/debug_parse.C Outdated Show resolved Hide resolved
parseAPI/src/debug_parse.C Outdated Show resolved Hide resolved
#pragma omp critical
{
v = fprintf(stderr, "[thread %d] ", id);
v &= vfprintf(stderr, format, va);
Copy link
Contributor

Choose a reason for hiding this comment

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

The &= doesn't make sense. fprintf returns the number of characters written or -1 on error. You should record each return value separately in r1 and r2 with a return value of r1 != -1 && r2 != -1) ? (r1 + r2) : -1. Slightly better would be to also only execute the second vprintf if r1 != -1. It is also likely that nothing looks at the return value.

Copy link
Contributor

@kupsch kupsch left a comment

Choose a reason for hiding this comment

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

slight change in error handling.

@hainest hainest merged commit b3a0d44 into master Dec 27, 2023
13 checks passed
@hainest hainest deleted the thaines/parseapi_parsing_printf branch December 27, 2023 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleanup Bring the code up to modern standards or remove deprecated features parseAPI This issue is directly related to parseAPI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants