Allow logging outside of setting the logging callback #106
Conversation
deschuma
commented
May 19, 2020
- Addresses Issue Enable logging if environment variable set #102 to enable logging to stdout independent of OE SDK
- Implements OE SDK ask to enable testing without on-demand registration, if desired
- Moves the environment variable functionality into a header file
…itly set AZDCAP logging to stdout
…, and cleanup logging
… a message indicating debug logging is enabled; Prefer logging callback over printf if it is set
src/environment.h
Outdated
| return std::string(); | ||
| } | ||
|
|
||
| auto retval = std::string(env_value); |
There was a problem hiding this comment.
Why save to a local variable here before returning? Why not just return std::string(env_value);?
| return std::string(); | ||
| } | ||
| #endif | ||
| else |
There was a problem hiding this comment.
else [](start = 4, length = 4)
This 'else' is unnecessary. Both 'if' statements exit the function. Visually, it seems odd to share 'else' block by 2 'if' blocks.
| { | ||
| log(SGX_QL_LOG_ERROR, | ||
| "Value specified in environment variable %s is either empty or " | ||
| "expected max length '%d'.", |
There was a problem hiding this comment.
%d [](start = 38, length = 2)
Missing an integer param for the %d.
|
The logging for parameters passed in sgx_ql_get_quote_config() is only on when the target was Not found in the cache. Could you take this chance to enable logging when cache was hit as well? |
src/logging.cpp
Outdated
| using namespace std; | ||
|
|
||
| sgx_ql_logging_function_t logger_callback = nullptr; | ||
| static sgx_ql_log_level_t debug_log_level; |
There was a problem hiding this comment.
debug_log_level [](start = 26, length = 15)
Initialize it. #Closed
src/logging.cpp
Outdated
| if (logger_callback != nullptr) | ||
| { | ||
| logger_callback(level, message); | ||
| }else if (enable_debug_log) |
src/logging.cpp
Outdated
|
|
||
| sgx_ql_logging_function_t logger_callback = nullptr; | ||
| static sgx_ql_log_level_t debug_log_level; | ||
| bool enable_debug_log = false; |
There was a problem hiding this comment.
enable_debug_log [](start = 5, length = 16)
Minor suggestion - enable_debug_log is always coupled with debug_log_level. You can define something like "SGX_QL_LOG_NONE" to replace enable_debug_log being false.
|
@soccerGB logging for parameters happens regardless of caching or not - is there a specific parameter I am missing? |