-
Notifications
You must be signed in to change notification settings - Fork 255
Add and use eprintf(), fprinte(), and eprinte() #1289
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
base: master
Are you sure you want to change the base?
Conversation
|
I recommend reviewing the second commit with $ git show HEAD^ --color-words=. |
5fa8f9a to
c3af808
Compare
|
I have some reservations about commits 1 and 2. Specifically, the new interface introduced isn't something I've commonly seen in other projects, even though I'm aware of its mention in the GCC documentation. From my perspective, it doesn't appear to offer a significant benefit here. Furthermore, I'm concerned that commit 2 introduces changes across many files, which could make it quite challenging for distribution maintainers to backport future changes to their distributions. This could lead to a lot of friction and messy patch applications down the line. On the other hand, I'm fine with the changes in commits 3 and 4. They look good to me. |
Yeah, I guess this would be a lot of work for distros. I'm fine not applying it for now; we can come back to it in the future.
I will split patches 3 and 4 to allow applying them separately. |
c3af808 to
a3ed1da
Compare
|
On the other hand, I plan to do a lot of work on error messages after this eprintf() patch, trying to unify them and use But yeah, we have lower hanging fruits for now. |
a3ed1da to
13a8030
Compare
|
I think we now have strong reasons to merge this. See the bug fixed in a call to SYSLOG() (and fprintf()). Edit: I've found more bugs after spending more time looking at these calls. |
97d7478 to
68dfb3a
Compare
55cd6cb to
0a20d7b
Compare
0a20d7b to
3b1b87e
Compare
v8
|
3b1b87e to
65c2e9c
Compare
85e28e2 to
3214071
Compare
|
v15
range-diff |
06ffd61 to
185fd35
Compare
185fd35 to
c1b93d2
Compare
The double parentheses were used to fake a variadic macro with a non-variadic one. With a variadic macro, we remove the weird double parentheses that were needed to call this macro, which BTW were error-prone. This would have prevented the bug fixed in 3f5ae5d (2025-09-10; "src/su.c: Fix incorrect (non-matching) parentheses"). Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
The name of this macro makes it more evident what it does. It's a C-locale version of syslog(3). We need to implement it as a macro so that arguments such as strerror(3) are evaluated after setting the C locale. This would be impossible with a function. Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
There's no need for two variables, and this avoids one assignment. Signed-off-by: Alejandro Colomar <alx@kernel.org>
…nally Both setlocale(3) and free(3) are okay to call with a null pointer. Signed-off-by: Alejandro Colomar <alx@kernel.org>
In some cases, we print strerrno() with this macro. Because this is a macro, and internal calls such as strdup(3) may set errno, that value could be corrupted when we arrive at syslog(3). While we could solve this here, it's not robust. Instead, we'll use a dedicated wrapper for that, which will be added in the following commits: SYSLOGE(). What we'll do here is preserve errno when we exit from this macro, as we often follow SYSLOG() calls with fprintf(stderr,) calls, which also use the errno value, and we don't want to pollute that. Signed-off-by: Alejandro Colomar <alx@kernel.org>
c1b93d2 to
60a5920
Compare
|
v16
range-diff |
@stoeckmann has provided a reproducer for a bug that we have as a plague: clobbering of errno. See: We need to solve this by having APIs that preserve errno. And this needs global churn, because the bug is everywhere.
|
Signed-off-by: Alejandro Colomar <alx@kernel.org>
These functions print a formatted string, followed by a colon and a space, followed by an error string produced by strerror($2). That is, the output is as follows: fmt: error message string For example, fprintec(stderr, ENOMEM, "foo(%d)", 42); prints foo(42): Cannot allocate memory These functions return the number of characters printed, or a negative value on error, like fprintf(3). Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
fprinte() is like fprintec(), but uses errno instead of an error code. It is implemented as a macro so that it can read the value in errno before evaluating any of its arguments, thus not corrupting it. It also preserves errno so that the error can be printed more than once. This is useful because we often print to stderr or log_get_logfd(), and immediately after print to the system log with syslog(3). Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
like the fprintf(3) wrappers, these print an error message. SYSLOGE() uses errno, and SYSLOGEC() uses an errno-like error code. This time we need to be careful to name the local copy of errno differently than within SYSLOG() --which we call--. Let's use a double underscore. In the future, C might have function literals (similar to lambdas), which will solve this issue. Signed-off-by: Alejandro Colomar <alx@kernel.org>
Having such long and complex format strings and variadic arguments is error-prone, as can be seen in the previous commit, which fixes a bug of this kind. Signed-off-by: Alejandro Colomar <alx@kernel.org>
This will allow having shorter lines for writing to stderr. This name is commonly used in other projects, it seems (see link below). Link: <https://gcc.gnu.org/onlinedocs/gcc-3.1.1/cpp/Variadic-Macros.html> Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
60a5920 to
08d4abe
Compare
This allows compacting a lot of code. It also makes it easier to discern lines that print to actual files, from those that print to stderr/stdout.
Queued after:
Revisions:
v2
#include "config.h"with quotes.v2b
v3
v4
v5
v5b
v6
v6b
v6c
v6d
v6e
v6f
v6g
v7
v8: #1289 (comment)
v8b
v9
v9b
v9c
v9d
v9e
v9f
v9g
v10: #1289 (comment)
v10b
v11
v12: #1289 (comment)
v12b
v12c
v12d
v13
v13b
v14
v15: #1289 (comment)
v15b
v15c
v16: #1289 (comment)
v16b