Commit e38fcb3a authored by Adam Simpkins's avatar Adam Simpkins Committed by Facebook Github Bot

logging: make XLOG_GET_CATEGORY() safe for all callers

Summary:
The `XLOG_GET_CATEGORY()` macro was previously written assuming it was only
used inside `XLOG()` statement.  When used inside the main translation unit
being compiled (e.g., a .cpp file and not a header file), the code assumed that
the file-scope category had already been initialized, since a level check had
presumably already been performed.

However, it is useful in some places for external users (outside of the logging
library itself) to call `XLOG_GET_CATEGORY()`.  In these cases a log level
check may not have been performed yet, so the file-scope category may not be
initialized yet.

This diff renames the existing `XLOG_GET_CATEGORY()` macro to
`XLOG_GET_CATEGORY_INTERNAL()` and adds a new `XLOG_GET_CATEGORY()` macro that
is slower (it always looks up the category by name) but always safe to use.

This also adds a new `XLOG_GET_CATEGORY_NAME()` macro, and renames the existing
`XLOG_SET_CATEGORY()` macro to `XLOG_SET_CATEGORY_NAME()` for API consistency.

Reviewed By: wez

Differential Revision: D5269975

fbshipit-source-id: 373805255823855282fa7a8d4a4d232ba06367f6
parent 440cddaf
...@@ -26,7 +26,7 @@ ...@@ -26,7 +26,7 @@
using namespace folly; using namespace folly;
using std::make_shared; using std::make_shared;
XLOG_SET_CATEGORY("xlog_test.main_file"); XLOG_SET_CATEGORY_NAME("xlog_test.main_file");
// Note that the XLOG* macros always use the main LoggerDB singleton. // Note that the XLOG* macros always use the main LoggerDB singleton.
// There is no way to get them to use a test LoggerDB during unit tests. // There is no way to get them to use a test LoggerDB during unit tests.
......
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include <folly/experimental/logging/LogStream.h> #include <folly/experimental/logging/LogStream.h>
#include <folly/experimental/logging/Logger.h> #include <folly/experimental/logging/Logger.h>
#include <folly/experimental/logging/LoggerDB.h> #include <folly/experimental/logging/LoggerDB.h>
#include <cstdlib>
/* /*
* This file contains the XLOG() and XLOGF() macros. * This file contains the XLOG() and XLOGF() macros.
...@@ -32,7 +33,7 @@ ...@@ -32,7 +33,7 @@
* "src.foo.bar" * "src.foo.bar"
* *
* If desired, the log category name used by XLOG() in a .cpp file may be * If desired, the log category name used by XLOG() in a .cpp file may be
* overridden using XLOG_SET_CATEGORY() macro. * overridden using XLOG_SET_CATEGORY_NAME() macro.
*/ */
/** /**
...@@ -115,7 +116,7 @@ ...@@ -115,7 +116,7 @@
(!XLOG_IS_ON_IMPL(level)) \ (!XLOG_IS_ON_IMPL(level)) \
? static_cast<void>(0) \ ? static_cast<void>(0) \
: ::folly::LogStreamProcessorT<::folly::isLogLevelFatal(level)>( \ : ::folly::LogStreamProcessorT<::folly::isLogLevelFatal(level)>( \
XLOG_GET_CATEGORY(), \ XLOG_GET_CATEGORY_INTERNAL(), \
(level), \ (level), \
__FILE__, \ __FILE__, \
__LINE__, \ __LINE__, \
...@@ -180,16 +181,34 @@ ...@@ -180,16 +181,34 @@
return _xlogLevelToCheck_ >= _xlogCurrentLevel_; \ return _xlogLevelToCheck_ >= _xlogCurrentLevel_; \
}()) }())
/**
* Get the name of the log category that will be used by XLOG() statements
* in this file.
*/
#define XLOG_GET_CATEGORY_NAME() getXlogCategoryName(__FILE__)
/** /**
* Get a pointer to the LogCategory that will be used by XLOG() statements in * Get a pointer to the LogCategory that will be used by XLOG() statements in
* this file. * this file.
* *
* This is just a small wrapper around a LoggerDB::getCategory() call.
* This must be implemented as a macro since it uses __FILE__, and that must
* expand to the correct filename based on where the macro is used.
*/
#define XLOG_GET_CATEGORY() \
folly::LoggerDB::get()->getCategory(XLOG_GET_CATEGORY_NAME())
/**
* Internal version of XLOG_GET_CATEGORY() that is used in the XLOG() macro.
*
* This macro is used in the XLOG() implementation, and therefore must be as * This macro is used in the XLOG() implementation, and therefore must be as
* cheap as possible. It stores the LogCategory* pointer as a local static * cheap as possible. It stores the LogCategory* pointer as a local static
* variable. Only the first invocation has to look up the log category by * variable. Only the first invocation has to look up the log category by
* name. Subsequent invocations re-use the already looked-up LogCategory * name. Subsequent invocations re-use the already looked-up LogCategory
* pointer. * pointer.
* *
* This is only safe to call after XlogLevelInfo::init() has been called.
*
* Most of this code must live in the macro definition itself, and cannot be * Most of this code must live in the macro definition itself, and cannot be
* moved into a helper function: The getXlogCategoryName() call must be made as * moved into a helper function: The getXlogCategoryName() call must be made as
* part of the macro expansion in order to work correctly. We also want to * part of the macro expansion in order to work correctly. We also want to
...@@ -198,7 +217,7 @@ ...@@ -198,7 +217,7 @@
* *
* See XlogCategoryInfo for the implementation details. * See XlogCategoryInfo for the implementation details.
*/ */
#define XLOG_GET_CATEGORY() \ #define XLOG_GET_CATEGORY_INTERNAL() \
[] { \ [] { \
static ::folly::XlogCategoryInfo<XLOG_IS_IN_HEADER_FILE> _xlogCategory_; \ static ::folly::XlogCategoryInfo<XLOG_IS_IN_HEADER_FILE> _xlogCategory_; \
if (!_xlogCategory_.isInitialized()) { \ if (!_xlogCategory_.isInitialized()) { \
...@@ -208,8 +227,8 @@ ...@@ -208,8 +227,8 @@
}() }()
/** /**
* XLOG_SET_CATEGORY() can be used to explicitly define the log category name * XLOG_SET_CATEGORY_NAME() can be used to explicitly define the log category
* used by all XLOG() and XLOGF() calls in this translation unit. * name used by all XLOG() and XLOGF() calls in this translation unit.
* *
* This overrides the default behavior of picking a category name based on the * This overrides the default behavior of picking a category name based on the
* current filename. * current filename.
...@@ -217,20 +236,20 @@ ...@@ -217,20 +236,20 @@
* This should be used at the top-level scope in a .cpp file, before any XLOG() * This should be used at the top-level scope in a .cpp file, before any XLOG()
* or XLOGF() macros have been used in the file. * or XLOGF() macros have been used in the file.
* *
* XLOG_SET_CATEGORY() cannot be used inside header files. * XLOG_SET_CATEGORY_NAME() cannot be used inside header files.
*/ */
#ifdef __INCLUDE_LEVEL__ #ifdef __INCLUDE_LEVEL__
#define XLOG_SET_CATEGORY(category) \ #define XLOG_SET_CATEGORY_NAME(category) \
namespace { \ namespace { \
static_assert( \ static_assert( \
__INCLUDE_LEVEL__ == 0, \ __INCLUDE_LEVEL__ == 0, \
"XLOG_SET_CATEGORY() should not be used in header files"); \ "XLOG_SET_CATEGORY_NAME() should not be used in header files"); \
inline std::string getXlogCategoryName(const char*) { \ inline std::string getXlogCategoryName(const char*) { \
return category; \ return category; \
} \ } \
} }
#else #else
#define XLOG_SET_CATEGORY(category) \ #define XLOG_SET_CATEGORY_NAME(category) \
namespace { \ namespace { \
inline std::string getXlogCategoryName(const char*) { \ inline std::string getXlogCategoryName(const char*) { \
return category; \ return category; \
...@@ -288,7 +307,7 @@ struct XlogLevelInfo { ...@@ -288,7 +307,7 @@ struct XlogLevelInfo {
template <bool IsInHeaderFile> template <bool IsInHeaderFile>
struct XlogCategoryInfo { struct XlogCategoryInfo {
public: public:
bool isInitialized() { bool isInitialized() const {
return isInitialized_.load(std::memory_order_acquire); return isInitialized_.load(std::memory_order_acquire);
} }
...@@ -339,16 +358,15 @@ struct XlogLevelInfo<false> { ...@@ -339,16 +358,15 @@ struct XlogLevelInfo<false> {
template <> template <>
struct XlogCategoryInfo<false> { struct XlogCategoryInfo<false> {
public: public:
constexpr bool isInitialized() { constexpr bool isInitialized() const {
// XlogLevelInfo<false>::check() is always called before XlogCategoryInfo // XlogLevelInfo<false>::init() is always called before XlogCategoryInfo
// is used, and it will will have already initialized fileScopeInfo. // is used, and it will have already initialized fileScopeInfo.
// Therefore we never have to check if it is initialized yet here. // Therefore we never have to check if it is initialized yet here.
return true; return true;
} }
LogCategory* init(folly::StringPiece) { [[noreturn]] LogCategory* init(folly::StringPiece /* categoryName */) {
// This method is never used given that isInitialized() always returns true // This method is never used given that isInitialized() always returns true
abort(); ::std::abort();
return nullptr;
} }
LogCategory* getCategory(XlogFileScopeInfo* fileScopeInfo) { LogCategory* getCategory(XlogFileScopeInfo* fileScopeInfo) {
return fileScopeInfo->category; return fileScopeInfo->category;
...@@ -360,7 +378,7 @@ struct XlogCategoryInfo<false> { ...@@ -360,7 +378,7 @@ struct XlogCategoryInfo<false> {
* Get the default XLOG() category name for the given filename. * Get the default XLOG() category name for the given filename.
* *
* This function returns the category name that will be used by XLOG() if * This function returns the category name that will be used by XLOG() if
* XLOG_SET_CATEGORY() has not been used. * XLOG_SET_CATEGORY_NAME() has not been used.
*/ */
std::string getXlogCategoryNameForFile(folly::StringPiece filename); std::string getXlogCategoryNameForFile(folly::StringPiece filename);
} }
...@@ -374,11 +392,11 @@ std::string getXlogCategoryNameForFile(folly::StringPiece filename); ...@@ -374,11 +392,11 @@ std::string getXlogCategoryNameForFile(folly::StringPiece filename);
namespace { namespace {
/** /**
* The default getXlogCategoryName() implementation. * The default getXlogCategoryName() implementation.
* This will be used if XLOG_SET_CATEGORY() has not been used yet. * This will be used if XLOG_SET_CATEGORY_NAME() has not been used yet.
* *
* This is a template purely so that XLOG_SET_CATEGORY() can define a more * This is a template purely so that XLOG_SET_CATEGORY_NAME() can define a more
* specific version if desired, allowing XLOG_SET_CATEGORY() to override this * specific version if desired, allowing XLOG_SET_CATEGORY_NAME() to override
* implementation once it has been used. The template paramete * this implementation once it has been used.
*/ */
template <typename T> template <typename T>
inline std::string getXlogCategoryName(const T* filename) { inline std::string getXlogCategoryName(const T* filename) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment