- char *esp_log_system_timestamp(void)
- {
- static char buffer[18] = {0};
- static _lock_t bufferLock = 0;
- if (xTaskGetSchedulerState() == taskSCHEDULER_NOT_STARTED) {
- // Omitted for clarity
- } else {
- // Get the local board time
- _lock_acquire(&bufferLock);
- snprintf(buffer, sizeof(buffer),
- "%02d:%02d:%02d.%03ld",
- timeinfo.tm_hour,
- timeinfo.tm_min,
- timeinfo.tm_sec,
- tv.tv_usec / 1000);
- _lock_release(&bufferLock);
- return buffer;
- }
- }
And here's the example of this function usage (the only one I could find across the code base):
- #elif CONFIG_LOG_TIMESTAMP_SOURCE_SYSTEM
- #define ESP_LOG_LEVEL(level, tag, format, ...) do { \
- if (level==ESP_LOG_ERROR ) { esp_log_write(ESP_LOG_ERROR, tag, LOG_SYSTEM_TIME_FORMAT(E, format), esp_log_system_timestamp(), tag, ##__VA_ARGS__); } \
- else if (level==ESP_LOG_WARN ) { esp_log_write(ESP_LOG_WARN, tag, LOG_SYSTEM_TIME_FORMAT(W, format), esp_log_system_timestamp(), tag, ##__VA_ARGS__); } \
- else if (level==ESP_LOG_DEBUG ) { esp_log_write(ESP_LOG_DEBUG, tag, LOG_SYSTEM_TIME_FORMAT(D, format), esp_log_system_timestamp(), tag, ##__VA_ARGS__); } \
- else if (level==ESP_LOG_VERBOSE ) { esp_log_write(ESP_LOG_VERBOSE, tag, LOG_SYSTEM_TIME_FORMAT(V, format), esp_log_system_timestamp(), tag, ##__VA_ARGS__); } \
- else { esp_log_write(ESP_LOG_INFO, tag, LOG_SYSTEM_TIME_FORMAT(I, format), esp_log_system_timestamp(), tag, ##__VA_ARGS__); } \
- } while(0)
- #endif //CONFIG_LOG_TIMESTAMP_SOURCE_xxx
I'm pretty sure that there's a data race here and the lock should be used outside this function call but maybe I am missing something here. It seems to me that the more robust solution will be either using mutex outside the function call (before and after if statement in the ESP_LOG_LEVEL macro) or making the buffer task local. Please correct me if I am wrong, I would appreciate any feedback on that.