-
Notifications
You must be signed in to change notification settings - Fork 82
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
Add newline to logs #307
Add newline to logs #307
Conversation
_Z_LOG_PREFIX(ERROR); \ | ||
printf(x, ##__VA_ARGS__); | ||
#define _Z_ERROR_CONTINUE(x, ...) printf(x, ##__VA_ARGS__); | ||
#define _Z_DEBUG(...) \ |
Check warning
Code scanning / Cppcheck (reported by Codacy)
misra violation 2101 with no text in the supplied rule-texts-file Warning
#define _Z_INFO(x, ...) (void)(0) | ||
#define _Z_ERROR(x, ...) (void)(0) | ||
#endif | ||
#define _Z_ERROR(...) \ |
Check warning
Code scanning / Cppcheck (reported by Codacy)
misra violation 2101 with no text in the supplied rule-texts-file Warning
#if ZENOH_DEBUG == 0 && !defined(Z_BUILD_DEBUG) | ||
|
||
#define _Z_DEBUG(...) (void)(0) | ||
#define _Z_INFO(...) (void)(0) |
Check warning
Code scanning / Cppcheck (reported by Codacy)
misra violation 2101 with no text in the supplied rule-texts-file Warning
@@ -17,55 +17,57 @@ | |||
|
|||
#include <stdio.h> | |||
|
|||
// Logging values | |||
#define _Z_LOG_LVL_ERROR 1 | |||
#define _Z_LOG_LVL_INFO 2 |
Check warning
Code scanning / Cppcheck (reported by Codacy)
misra violation 2101 with no text in the supplied rule-texts-file Warning
// Ignore print only if log deactivated and build is release | ||
#if ZENOH_DEBUG == 0 && !defined(Z_BUILD_DEBUG) | ||
|
||
#define _Z_DEBUG(...) (void)(0) |
Check warning
Code scanning / Cppcheck (reported by Codacy)
misra violation 2101 with no text in the supplied rule-texts-file Warning
_Z_LOG_PREFIX(ERROR); \ | ||
printf(x, ##__VA_ARGS__); | ||
#define _Z_ERROR_CONTINUE(x, ...) printf(x, ##__VA_ARGS__); | ||
#define _Z_INFO(...) \ |
Check warning
Code scanning / Cppcheck (reported by Codacy)
misra violation 2101 with no text in the supplied rule-texts-file Warning
|
||
#define _Z_DEBUG(...) (void)(0) | ||
#define _Z_INFO(...) (void)(0) | ||
#define _Z_ERROR(...) (void)(0) |
Check warning
Code scanning / Cppcheck (reported by Codacy)
misra violation 2101 with no text in the supplied rule-texts-file Warning
printf(x, ##__VA_ARGS__); | ||
#define _Z_ERROR_CONTINUE(x, ...) printf(x, ##__VA_ARGS__); | ||
// Ignore print only if log deactivated and build is release | ||
#if ZENOH_DEBUG == 0 && !defined(Z_BUILD_DEBUG) |
Check warning
Code scanning / Cppcheck (reported by Codacy)
misra violation 2009 with no text in the supplied rule-texts-file Warning
@@ -17,55 +17,57 @@ | |||
|
|||
#include <stdio.h> | |||
|
|||
// Logging values | |||
#define _Z_LOG_LVL_ERROR 1 |
Check warning
Code scanning / Cppcheck (reported by Codacy)
misra violation 2101 with no text in the supplied rule-texts-file Warning
// Logging values | ||
#define _Z_LOG_LVL_ERROR 1 | ||
#define _Z_LOG_LVL_INFO 2 | ||
#define _Z_LOG_LVL_DEBUG 3 |
Check warning
Code scanning / Cppcheck (reported by Codacy)
misra violation 2101 with no text in the supplied rule-texts-file Warning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some clarification required.
printf(__VA_ARGS__); \ | ||
printf("\n"); \ | ||
} \ | ||
} while (false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for this do..while
if the loop only executes once?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turns the block into a single statement and forces a ;
after the macro. If you want more info on this: https://stackoverflow.com/questions/4674480/do-whilefalse-pattern
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If MISRA-C is still followed for the codebase, this shall not be required.
But I get the point...
#define _Z_ERROR_CONTINUE(x, ...) printf(x, ##__VA_ARGS__); | ||
#define _Z_DEBUG(...) \ | ||
do { \ | ||
if (ZENOH_DEBUG >= _Z_LOG_LVL_DEBUG) { \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why to do this check at runtime, instead of at compile time as before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually it transitions from a pre-compiler thing to a compile time check, as the compiler realises this values won't change for the whole execution of the program, a bit like a C++ constexpr
. But just to be extra sure I added the (void)(0)
in release mode and no debug, in case some exotic compiler emits run time checks.
The problem with pre compiler stuff is that it doesn't check if the code is valid while the compiler do. So let's say in my log statement I print a variable, that is then changed (type or name), it will go unnoticed until the compiler sees it, so if I only compile without Debug it won't see it. Now it will always see this kind of issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if it is already in place, but having compilation tests for both debug and release seems a simpler option.
5a236c8
to
4414494
Compare
4414494
to
2313ad1
Compare
This fixes #238
In addition, it switches to a compiler based macro, to ensure no obsolete log will blow up 2 years after the renaming of a variable because we didn't build with the correct
ZENOH_DEBUG
value.Also added a compiler symbol to identify a debug/release build.