Copilot commented on code in PR #13262:
URL: https://github.com/apache/trafficserver/pull/13262#discussion_r3408415128
##########
include/tscore/DiagsTypes.h:
##########
@@ -45,111 +45,351 @@
#include "tscore/ink_mutex.h"
#include "tsutil/Regex.h"
+/// Sentinel value stored in Diags::magic to detect heap corruption or
+/// use-after-free of a Diags instance. Diags::magic == DIAGS_MAGIC for
+/// any properly constructed, living instance.
#define DIAGS_MAGIC 0x12345678
-#define BYTES_IN_MB 1000000
-enum DiagsTagType {
- DiagsTagType_Debug = 0, // do not renumber --- used as array index
- DiagsTagType_Action = 1
-};
+/// Bytes-per-megabyte (decimal, not binary). Used by configuration
+/// parsing to convert MB-valued knobs into byte counts.
+#define BYTES_IN_MB 1000000
+/**
+ * @brief Selects which of the two tag tables a Diags operation addresses:
+ * debug tags (controlling Dbg/Diag emission) or action tags (controlling
+ * is_action_tag_set conditional code paths).
+ *
+ * @note Numeric values are used as array indices into the enable-state
+ * array and Diags::activated_tags. Do not renumber.
+ */
+enum DiagsTagType { DiagsTagType_Debug = 0, DiagsTagType_Action = 1 };
+
+/**
+ * @brief Per-DiagsLevel output destination configuration.
+ *
+ * Each boolean controls whether messages at the owning level are emitted
+ * to the corresponding sink. Multiple sinks may be set simultaneously;
+ * the message is emitted to all enabled sinks in unspecified order.
+ *
+ * @par Thread safety
+ * Carries no internal synchronization. Callers reading or
+ * modifying Diags::config.outputs[] must respect the concurrency contract
+ * of the containing DiagsConfigState.
+ */
struct DiagsModeOutput {
bool to_stdout;
bool to_stderr;
bool to_syslog;
bool to_diagslog;
};
+/**
+ * @brief Identifies which standard stream Diags::set_std_output reseats.
+ *
+ * STDOUT targets the standard output stream; STDERR targets the standard
+ * error stream.
+ */
enum StdStream { STDOUT = 0, STDERR };
+/**
+ * @brief Selects the rolling policy for a managed log file.
+ *
+ * - NO_ROLLING — never roll; file grows without bound.
+ * - ROLL_ON_TIME — roll when the configured time interval elapses.
+ * - ROLL_ON_SIZE — roll when the configured size threshold is
exceeded.
+ * - ROLL_ON_TIME_OR_SIZE — roll when either condition is satisfied.
+ * - INVALID_ROLLING_VALUE — sentinel; any code path receiving this value MUST
+ * treat it as NO_ROLLING. MUST be the largest enumerator so that range
checks
+ * can detect out-of-range configuration values.
+ */
enum RollingEnabledValues { NO_ROLLING = 0, ROLL_ON_TIME, ROLL_ON_SIZE,
ROLL_ON_TIME_OR_SIZE, INVALID_ROLLING_VALUE };
+/// Count of valid DiagsLevel values; equal to DL_Undefined and used to
+/// size DiagsConfigState::outputs[].
#define DiagsLevel_Count DL_Undefined
+/// True if the given DiagsLevel causes process termination after emission.
+/// Terminal levels are DL_Fatal and above (excluding DL_Undefined).
#define DiagsLevel_IsTerminal(_l) (((_l) >= DL_Fatal) && ((_l) < DL_Undefined))
-// Cleanup Function Prototype - Called before ink_fatal to
-// cleanup process state
+/**
+ * @brief Cleanup callback invoked before process termination on a terminal
+ * DiagsLevel (DL_Fatal, DL_Alert, DL_Emergency).
+ *
+ * @pre Runs synchronously on the thread that emitted the terminal message,
+ * before the process exits. MUST NOT throw. MUST be async-signal-safe if
+ * installed in a process that may emit terminal-level messages from a
+ * signal handler. MUST tolerate being invoked more than once per process
+ * (for example, from concurrent terminal emissions on different threads,
+ * or from a terminal emission inside the callback itself).
+ * @post On return, the process is terminated; control does not return to
+ * the emitter. DL_Emergency exits via the unrecoverable path; DL_Fatal
+ * and DL_Alert exit via the recoverable path.
+ * @par Thread safety
+ * No serialization is provided. The callback may run concurrently on
+ * multiple threads if multiple terminal emissions race; the callback
+ * itself is responsible for any required synchronization or idempotency.
+ */
using DiagsCleanupFunc = void (*)();
+/**
+ * @brief Bundles the two orthogonal pieces of Diags output configuration:
+ * the process-global per-tag enable state and the per-level output routing.
+ *
+ * The enable state for each DiagsTagType is one of:
+ * 0 — disabled (no emission),
+ * 1 — enabled for all callers,
+ * 2 — enabled only when ContFlags::DEBUG_OVERRIDE is set on the
+ * current Continuation (per-connection debug override),
+ * 3 — treated identically to 1 (always-enabled).
+ *
+ * The output routing is stored in @c outputs[], one entry per DiagsLevel.
+ *
+ * @par Thread safety
+ * The enable state is process-global (static storage). Reads via
+ * enabled() and writes via enabled(dtt, value) carry no internal
+ * synchronization; callers must provide external serialization when a
+ * write may race a read. The @c outputs[] array carries no internal
+ * synchronization either; the same serialization requirement applies.
+ */
class DiagsConfigState
{
public:
+ /**
+ * @brief Return the current enable state for the given tag type.
+ *
+ * @param[in] dtt DiagsTagType_Debug or DiagsTagType_Action.
+ * @return 0 (disabled), 1 (enabled), 2 (DEBUG_OVERRIDE mode), or 3
+ * (always-enabled, same effect as 1).
+ * @pre None.
+ * @post No state change.
+ * @par Errors
+ * None.
+ * @par Thread safety
+ * Not thread-safe. The read carries no synchronization;
+ * concurrent writes from a reconfiguration thread are a data race.
+ */
static int
enabled(DiagsTagType dtt)
{
return _enabled[dtt];
}
+ /**
+ * @brief Set the enable state for the given tag type.
+ *
+ * @param[in] dtt DiagsTagType_Debug or DiagsTagType_Action.
+ * @param[in] new_value 0, 1, 2, or 3 (see class contract); 3 behaves
+ * identically to 1.
+ * @pre new_value is in [0, 3]; values outside this range produce
+ * unspecified behavior.
+ * @post enabled(dtt) returns new_value on all subsequent calls. When
+ * dtt == DiagsTagType_Debug and the value changes, the DbgCtl fast-path
+ * enable state is updated via a relaxed atomic store so that DbgCtl-based
+ * checks remain in sync without acquiring a lock.
+ * @par Errors
+ * None.
+ * @par Thread safety
+ * Must be called with external serialization against
+ * concurrent enabled(dtt) reads.
+ */
static void enabled(DiagsTagType dtt, int new_value);
- DiagsModeOutput outputs[DiagsLevel_Count]; // where each level prints
+ /// Per-level output routing. Each element controls where messages at
+ /// the corresponding DiagsLevel are sent. See DiagsModeOutput.
+ DiagsModeOutput outputs[DiagsLevel_Count];
private:
static int _enabled[2]; // one debug, one action
};
-//////////////////////////////////////////////////////////////////////////////
-//
-// class Diags
-//
-// The Diags class is used for global configuration of the run-time
-// diagnostics system. This class provides the following services:
-//
-// * run-time notices, debugging, warnings, errors
-// * debugging tags to selectively enable & disable diagnostics
-// * action tags to selectively enable & disable code paths
-// * configurable output to stdout, stderr, syslog, error logs
-// * interface to supporting on-the-fly reconfiguration
-//
-//////////////////////////////////////////////////////////////////////////////
-
+/**
+ * @brief Active diagnostic emission and tag-table state for the process.
+ *
+ * Owns the diags.log, stdout, and stderr BaseLogFile handles, the per-level
+ * output routing configuration, the regex tables for debug and action tags,
+ * and the rolling policy for the diagnostic and standard-output logs.
+ * All diagnostic emission macros (Status, Note, Warning, Error, Fatal, Alert,
+ * Emergency, Diag) route through the process-global Diags instance installed
+ * via DiagsPtr.
+ *
+ * @par Thread safety
+ * Concurrent emission calls (print, log, error) hold tag_table_lock for
+ * diagslog, stdout, and stderr output. On non-FreeBSD platforms the lock is
+ * released before the syslog() call, so concurrent syslog output is not
+ * serialized against other emission calls. Reconfiguration writes to
+ * config.outputs do not acquire tag_table_lock, so concurrent emission
+ * and reconfiguration are a data race on config.outputs. Tag-table
+ * mutation methods (activate_taglist,
+ * deactivate_all) acquire tag_table_lock internally. Log-pointer swap
methods
+ * (reseat_diagslog, should_roll_*, set_std_output) acquire tag_table_lock
+ * only for their pointer swap step; file operations run outside the lock.
Review Comment:
The class-level thread-safety note says these methods take tag_table_lock
only for a pointer swap and that file operations run outside the lock, but
set_std_output() also performs the dup2() rebinding under tag_table_lock.
Please adjust this documentation so it matches the actual locking behavior.
##########
include/tscore/DiagsTypes.h:
##########
@@ -174,12 +441,25 @@ class Diags : public DebugInterface
// raw printing interfaces //
/////////////////////////////
- ///////////////////////////////////////////////////////////////////////
- // user diagnostic output interfaces --- enabled on or off based //
- // on the value of the enable flag, and the state of the debug tags. //
- ///////////////////////////////////////////////////////////////////////
-
- /// Print the log message without respect to whether the tag is enabled.
+ /**
+ * @brief Emit a message unconditionally, regardless of tag state.
+ *
+ * @param[in] tag C string label included in output, or nullptr to omit the
+ * tag prefix. Not checked against the tag regex.
+ * @param[in] level DiagsLevel for sink routing.
+ * @param[in] loc Source location of the call site, or nullptr.
+ * @param[in] fmt Non-null printf-format string.
+ * @pre fmt is non-null and arguments match its conversions.
+ * @post Message emitted to all sinks enabled for level in config.outputs.
+ * stderr is also written when regression_testing_on is true, even if
+ * config.outputs[level].to_stderr is false.
+ * Does not handle terminal levels; the process does not exit.
+ * Use error_va() if terminal-level exit behavior is required.
+ * @par Errors
+ * I/O errors during emission are absorbed; output may be lost.
+ * @par Thread safety
+ * Safe to call concurrently from any thread.
Review Comment:
This method documentation says print() is safe to call concurrently from any
thread, but concurrent reconfiguration writes to config.outputs are explicitly
a data race (see DiagsConfig::reconfigure_diags()). Please qualify the
thread-safety statement so it doesn't promise safety against concurrent config
updates.
##########
include/tscore/DiagsTypes.h:
##########
@@ -220,26 +562,286 @@ class Diags : public DebugInterface
va_end(ap);
}
+ /**
+ * @brief va_list form of error().
+ *
+ * @param[in] level DiagsLevel for routing and terminal handling.
+ * @param[in] loc Source location, or nullptr.
+ * @param[in] fmt Non-null printf-format string.
+ * @param[in] ap Initialized va_list. Consumed; caller MUST NOT reuse
+ * without va_end + va_start.
+ * @pre fmt is non-null; ap is initialized.
+ * @post Message emitted to all sinks enabled for level. For terminal
+ * levels (DL_Fatal and above), invokes cleanup_func (if set) and then
+ * terminates the process; does not return.
+ * @par Errors
+ * None signaled; I/O errors absorbed.
+ * @par Thread safety
+ * Safe to call concurrently from any thread.
+ */
virtual void error_va(DiagsLevel level, const SourceLocation *loc, const
char *fmt, va_list ap) const;
+ /**
+ * @brief Print the current Diags configuration to fp.
+ *
+ * @param[in] fp Destination FILE *; defaults to stdout.
+ * @pre fp is a valid open stream.
+ * @post Configuration summary written to fp.
+ * @par Errors
+ * I/O errors on fp are not signaled.
+ * @par Thread safety
+ * Not thread-safe. Reads config fields (enable state,
+ * outputs, base_debug_tags, base_action_tags) without holding
+ * tag_table_lock. Concurrent reconfiguration may produce
+ * interleaved or inconsistent output.
+ */
void dump(FILE *fp = stdout) const;
+ /**
+ * @brief Enable tags matching the given PCRE2 pattern for the given mode.
+ *
+ * @param[in] taglist PCRE2 regex string, or nullptr. If nullptr, the stored
+ * pattern is unchanged.
+ * @param[in] mode DiagsTagType_Debug or DiagsTagType_Action.
+ * @pre None (null taglist and invalid PCRE2 patterns are both safe; see
+ * @post for the invalid-pattern case). The regex engine is PCRE2, not
+ * POSIX ERE.
+ * @post If taglist is non-null, the new pattern unconditionally replaces the
+ * previous one; if the pattern fails to compile, the stored pattern
becomes
+ * empty and no tags will match until a valid pattern is installed.
+ * For DiagsTagType_Debug, if this is the process-global Diags instance,
+ * DbgCtl::update() is called to resync all registered debug controls
+ * against the current pattern — this occurs regardless of whether taglist
+ * is null.
+ * @par Errors
+ * Compile failures are not signaled to the caller; the stored pattern
+ * becomes empty rather than retaining the previous one.
+ * @par Thread safety
+ * Acquires tag_table_lock. Safe to call concurrently with
+ * emission, but serialized with other reconfiguration methods.
+ */
void activate_taglist(const char *taglist, DiagsTagType mode =
DiagsTagType_Debug);
+ /**
+ * @brief Disable all tags for the given mode.
+ *
+ * @param[in] mode DiagsTagType_Debug or DiagsTagType_Action.
+ * @pre None.
+ * @post No tag matches for mode; activated_tags[mode] is null.
+ * @par Errors
+ * None.
+ * @par Thread safety
+ * Acquires tag_table_lock. Safe to call concurrently with
+ * emission, but serialized with other reconfiguration methods.
+ */
void deactivate_all(DiagsTagType mode = DiagsTagType_Debug);
+ /**
+ * @brief Open the given BaseLogFile for use as a diagnostic log destination.
+ *
+ * Does NOT assign diags_log; the caller is responsible for that assignment
+ * on success. On failure, blf is deleted before returning.
+ *
+ * @param[in] blf Pointer to a constructed but not-yet-opened BaseLogFile,
+ * or nullptr (null is a no-op that returns true). When non-null, ownership
+ * transfers unconditionally: on success the caller must assign blf to
+ * diags_log; on failure blf has been deleted.
+ * @return True if blf was opened successfully (or blf is nullptr); false if
+ * the open failed (blf is deleted before returning false).
+ * @pre blf is null or points to a BaseLogFile that has not yet been opened.
+ * @post On true return: blf (if non-null) is open; caller must assign it to
+ * diags_log. On false return: blf has been deleted; diags_log is
unchanged.
+ * @par Errors
+ * Open failures cause a false return. A log_log_error diagnostic at
+ * LL_Error is emitted only when BASELOGFILE_DEBUG_MODE is enabled
+ * (off by default).
+ * @par Thread safety
+ * Caller MUST serialize with all other reconfiguration
+ * methods. reseat_diagslog() acquires tag_table_lock only for the
+ * pointer swap that follows this call; setup_diagslog() itself is not
+ * called under the lock. Direct callers must provide equivalent
+ * serialization for the pointer swap.
+ */
bool setup_diagslog(BaseLogFile *blf);
+
+ /**
+ * @brief Configure the rolling policy for diags.log.
+ *
+ * @param[in] re Rolling policy (see RollingEnabledValues). NO_ROLLING
+ * disables rolling.
+ * @param[in] ri Rolling interval in seconds, consulted under time-based
+ * policies. A value of -1 disables the time-based threshold.
+ * @param[in] rs Rolling size threshold in megabytes, consulted under
+ * size-based policies. A value of -1 disables the size-based threshold.
+ * @pre None.
+ * @post The three rolling policy fields are overwritten with the supplied
+ * values. No other state is modified.
+ * @par Errors
+ * None. Inputs are stored verbatim without validation.
+ * @par Thread safety
+ * No lock is acquired. The caller must ensure no concurrent
+ * calls to should_roll_diagslog() occur during reconfiguration.
+ */
void config_roll_diagslog(RollingEnabledValues re, int ri, int rs);
+
+ /**
+ * @brief Configure the rolling policy for the output log (traffic.out).
+ *
+ * @param[in] re Rolling policy (see RollingEnabledValues). NO_ROLLING
+ * disables rolling.
+ * @param[in] ri Rolling interval in seconds, consulted under time-based
+ * policies. A value of -1 disables the time-based threshold.
+ * @param[in] rs Rolling size threshold in megabytes, consulted under
+ * size-based policies. A value of -1 disables the size-based threshold.
+ * @pre None.
+ * @post The three rolling policy fields are overwritten with the supplied
+ * values. No other state is modified.
+ * @par Errors
+ * None. Inputs are stored verbatim without validation.
+ * @par Thread safety
+ * No lock is acquired. The caller must ensure no concurrent
+ * calls to should_roll_outputlog() occur during reconfiguration.
+ */
void config_roll_outputlog(RollingEnabledValues re, int ri, int rs);
+
+ /**
+ * @brief Close the current diags.log and reopen it at the configured path.
+ *
+ * Intended for use after an external log rotation tool has renamed or
+ * removed the active diags.log. The implementation:
+ * 1. Flushes the current file's buffered output.
+ * 2. Captures the configured filename from the active BaseLogFile.
+ * 3. Constructs a new BaseLogFile at that filename. The filename is
+ * passed to the OS verbatim — symlinks are re-resolved at each call.
+ * 4. On success: atomically swaps the new file into place under
+ * tag_table_lock and destroys the previous BaseLogFile (closing its
+ * FILE *).
+ * 5. On failure: leaves the previous file in place.
+ *
+ * @return False if diags_log is null or not yet initialized (safe no-op).
+ * True in all other cases, including when the internal reopen fails —
+ * reopen failures are not reflected in the return value.
+ * @pre A Diags instance is active and diags_log is initialized
+ * (is_init() == true). If this precondition is not met, returns false
+ * without performing a reopen — this is the safe no-op path for calls
+ * that arrive before the diagnostics subsystem is initialized.
+ * @post On success: diags_log is a fresh BaseLogFile at the configured
+ * path; the previous BaseLogFile (and its FILE *) is destroyed; all
+ * subsequent emission goes to the new file.
+ * On failure: diags_log is unchanged; the newly allocated BaseLogFile
+ * is deleted before it returns.
+ * @par Errors
+ * Open failures are not signaled via the return value. They are reported
+ * at LL_Error only when BASELOGFILE_DEBUG_MODE is enabled (off by
+ * default); in normal builds the failure is completely silent.
+ * @par Thread safety
+ * The swap in step 4 is performed under tag_table_lock.
+ * Concurrent emission either observes the pre-swap or post-swap log;
+ * no message is lost or written to a destroyed FILE *.
+ * @note When diagnostic output is disabled or redirected to a non-file
+ * sink (e.g., syslog), diags_log is null or uninitialized and the
+ * initialization guard causes an early false return.
+ */
bool reseat_diagslog();
+
+ /**
+ * @brief Roll diags.log if the current rolling policy condition is met.
+ *
+ * Under a size-based policy, the current file's size is compared to the
+ * configured threshold. Under a time-based policy, the elapsed time since
+ * the last roll is compared to the configured interval. Combined policies
+ * evaluate both conditions independently. When a condition is met the
+ * current file is flushed, renamed, and a fresh file is opened at the same
+ * path.
+ *
+ * @return True if the underlying file was renamed; false otherwise. True is
+ * returned even when reopening the fresh file at the original path fails.
+ * @pre None. A null or uninitialized diags_log is a safe no-op that
+ * returns false.
+ * @post On a successful roll the file at the configured path is a fresh,
+ * empty file and emission is directed to it. If renaming succeeded but
+ * reopening did not, the configured path is left absent and emission
+ * continues to the renamed file. With no roll, all state is unchanged.
+ * @par Errors
+ * None signaled. fstat (size-based policy only) and reopen failures
+ * silently suppress replacement; a completed rename is never reversed.
+ * @par Thread safety
+ * tag_table_lock is acquired only during the BaseLogFile
+ * pointer swap. The rolling-condition checks and file operations execute
+ * without a lock; the caller must ensure no concurrent reconfiguration
+ * (see config_roll_diagslog()).
+ */
bool should_roll_diagslog();
+
+ /**
+ * @brief Roll the standard-output redirection file if the current rolling
+ * policy condition is met.
+ *
+ * Under a size-based policy, the current file's size is compared to the
+ * configured threshold. Under a time-based policy, the elapsed time since
+ * the last roll is compared to the configured interval. Combined policies
+ * evaluate both conditions independently. When a condition is met both
+ * stdout_log and stderr_log are flushed, the underlying file is renamed,
+ * and a fresh file is opened at the same path; the process's stdout (and
+ * stderr, when redirected to the same path) are rebound to the new file.
+ * The time-based path also advances the last-roll timestamp.
+ *
+ * @return True if the underlying file was renamed; false otherwise.
+ * @pre stdout_log and stderr_log are non-null; violation aborts.
+ * When a roll occurs, stdout_log and stderr_log must refer to the same
+ * filesystem path; violation aborts.
+ * @post On a successful roll the file at the configured path is a fresh,
+ * empty file and the process's stdout and stderr (when sharing the path)
+ * are bound to it. With no roll, all state is unchanged. An
+ * uninitialized stdout_log returns false without inspecting policy.
Review Comment:
The `@post` implies that after a roll the code always reopens/binds a fresh
file at the original path. However, should_roll_outputlog() doesn’t check
set_std_output()’s return value, and set_std_output() can fail (leaving the
original path absent and stdout/stderr still bound to the renamed file). Please
document the reopen-failure behavior similarly to should_roll_diagslog().
##########
include/tscore/DiagsTypes.h:
##########
@@ -220,26 +562,286 @@ class Diags : public DebugInterface
va_end(ap);
}
+ /**
+ * @brief va_list form of error().
+ *
+ * @param[in] level DiagsLevel for routing and terminal handling.
+ * @param[in] loc Source location, or nullptr.
+ * @param[in] fmt Non-null printf-format string.
+ * @param[in] ap Initialized va_list. Consumed; caller MUST NOT reuse
+ * without va_end + va_start.
+ * @pre fmt is non-null; ap is initialized.
+ * @post Message emitted to all sinks enabled for level. For terminal
+ * levels (DL_Fatal and above), invokes cleanup_func (if set) and then
+ * terminates the process; does not return.
+ * @par Errors
+ * None signaled; I/O errors absorbed.
+ * @par Thread safety
+ * Safe to call concurrently from any thread.
+ */
virtual void error_va(DiagsLevel level, const SourceLocation *loc, const
char *fmt, va_list ap) const;
+ /**
+ * @brief Print the current Diags configuration to fp.
+ *
+ * @param[in] fp Destination FILE *; defaults to stdout.
+ * @pre fp is a valid open stream.
+ * @post Configuration summary written to fp.
+ * @par Errors
+ * I/O errors on fp are not signaled.
+ * @par Thread safety
+ * Not thread-safe. Reads config fields (enable state,
+ * outputs, base_debug_tags, base_action_tags) without holding
+ * tag_table_lock. Concurrent reconfiguration may produce
+ * interleaved or inconsistent output.
+ */
void dump(FILE *fp = stdout) const;
+ /**
+ * @brief Enable tags matching the given PCRE2 pattern for the given mode.
+ *
+ * @param[in] taglist PCRE2 regex string, or nullptr. If nullptr, the stored
+ * pattern is unchanged.
+ * @param[in] mode DiagsTagType_Debug or DiagsTagType_Action.
+ * @pre None (null taglist and invalid PCRE2 patterns are both safe; see
+ * @post for the invalid-pattern case). The regex engine is PCRE2, not
+ * POSIX ERE.
+ * @post If taglist is non-null, the new pattern unconditionally replaces the
+ * previous one; if the pattern fails to compile, the stored pattern
becomes
+ * empty and no tags will match until a valid pattern is installed.
+ * For DiagsTagType_Debug, if this is the process-global Diags instance,
+ * DbgCtl::update() is called to resync all registered debug controls
+ * against the current pattern — this occurs regardless of whether taglist
+ * is null.
+ * @par Errors
+ * Compile failures are not signaled to the caller; the stored pattern
+ * becomes empty rather than retaining the previous one.
+ * @par Thread safety
+ * Acquires tag_table_lock. Safe to call concurrently with
+ * emission, but serialized with other reconfiguration methods.
+ */
void activate_taglist(const char *taglist, DiagsTagType mode =
DiagsTagType_Debug);
+ /**
+ * @brief Disable all tags for the given mode.
+ *
+ * @param[in] mode DiagsTagType_Debug or DiagsTagType_Action.
+ * @pre None.
+ * @post No tag matches for mode; activated_tags[mode] is null.
+ * @par Errors
+ * None.
+ * @par Thread safety
+ * Acquires tag_table_lock. Safe to call concurrently with
+ * emission, but serialized with other reconfiguration methods.
+ */
void deactivate_all(DiagsTagType mode = DiagsTagType_Debug);
+ /**
+ * @brief Open the given BaseLogFile for use as a diagnostic log destination.
+ *
+ * Does NOT assign diags_log; the caller is responsible for that assignment
+ * on success. On failure, blf is deleted before returning.
+ *
+ * @param[in] blf Pointer to a constructed but not-yet-opened BaseLogFile,
+ * or nullptr (null is a no-op that returns true). When non-null, ownership
+ * transfers unconditionally: on success the caller must assign blf to
+ * diags_log; on failure blf has been deleted.
+ * @return True if blf was opened successfully (or blf is nullptr); false if
+ * the open failed (blf is deleted before returning false).
+ * @pre blf is null or points to a BaseLogFile that has not yet been opened.
+ * @post On true return: blf (if non-null) is open; caller must assign it to
+ * diags_log. On false return: blf has been deleted; diags_log is
unchanged.
+ * @par Errors
+ * Open failures cause a false return. A log_log_error diagnostic at
+ * LL_Error is emitted only when BASELOGFILE_DEBUG_MODE is enabled
+ * (off by default).
+ * @par Thread safety
+ * Caller MUST serialize with all other reconfiguration
+ * methods. reseat_diagslog() acquires tag_table_lock only for the
+ * pointer swap that follows this call; setup_diagslog() itself is not
+ * called under the lock. Direct callers must provide equivalent
+ * serialization for the pointer swap.
+ */
bool setup_diagslog(BaseLogFile *blf);
+
+ /**
+ * @brief Configure the rolling policy for diags.log.
+ *
+ * @param[in] re Rolling policy (see RollingEnabledValues). NO_ROLLING
+ * disables rolling.
+ * @param[in] ri Rolling interval in seconds, consulted under time-based
+ * policies. A value of -1 disables the time-based threshold.
+ * @param[in] rs Rolling size threshold in megabytes, consulted under
+ * size-based policies. A value of -1 disables the size-based threshold.
+ * @pre None.
+ * @post The three rolling policy fields are overwritten with the supplied
+ * values. No other state is modified.
+ * @par Errors
+ * None. Inputs are stored verbatim without validation.
+ * @par Thread safety
+ * No lock is acquired. The caller must ensure no concurrent
+ * calls to should_roll_diagslog() occur during reconfiguration.
+ */
void config_roll_diagslog(RollingEnabledValues re, int ri, int rs);
+
+ /**
+ * @brief Configure the rolling policy for the output log (traffic.out).
+ *
+ * @param[in] re Rolling policy (see RollingEnabledValues). NO_ROLLING
+ * disables rolling.
+ * @param[in] ri Rolling interval in seconds, consulted under time-based
+ * policies. A value of -1 disables the time-based threshold.
+ * @param[in] rs Rolling size threshold in megabytes, consulted under
+ * size-based policies. A value of -1 disables the size-based threshold.
+ * @pre None.
+ * @post The three rolling policy fields are overwritten with the supplied
+ * values. No other state is modified.
+ * @par Errors
+ * None. Inputs are stored verbatim without validation.
+ * @par Thread safety
+ * No lock is acquired. The caller must ensure no concurrent
+ * calls to should_roll_outputlog() occur during reconfiguration.
+ */
void config_roll_outputlog(RollingEnabledValues re, int ri, int rs);
+
+ /**
+ * @brief Close the current diags.log and reopen it at the configured path.
+ *
+ * Intended for use after an external log rotation tool has renamed or
+ * removed the active diags.log. The implementation:
+ * 1. Flushes the current file's buffered output.
+ * 2. Captures the configured filename from the active BaseLogFile.
+ * 3. Constructs a new BaseLogFile at that filename. The filename is
+ * passed to the OS verbatim — symlinks are re-resolved at each call.
+ * 4. On success: atomically swaps the new file into place under
+ * tag_table_lock and destroys the previous BaseLogFile (closing its
+ * FILE *).
+ * 5. On failure: leaves the previous file in place.
+ *
+ * @return False if diags_log is null or not yet initialized (safe no-op).
+ * True in all other cases, including when the internal reopen fails —
+ * reopen failures are not reflected in the return value.
+ * @pre A Diags instance is active and diags_log is initialized
+ * (is_init() == true). If this precondition is not met, returns false
+ * without performing a reopen — this is the safe no-op path for calls
+ * that arrive before the diagnostics subsystem is initialized.
+ * @post On success: diags_log is a fresh BaseLogFile at the configured
+ * path; the previous BaseLogFile (and its FILE *) is destroyed; all
+ * subsequent emission goes to the new file.
+ * On failure: diags_log is unchanged; the newly allocated BaseLogFile
+ * is deleted before it returns.
+ * @par Errors
+ * Open failures are not signaled via the return value. They are reported
+ * at LL_Error only when BASELOGFILE_DEBUG_MODE is enabled (off by
+ * default); in normal builds the failure is completely silent.
+ * @par Thread safety
+ * The swap in step 4 is performed under tag_table_lock.
+ * Concurrent emission either observes the pre-swap or post-swap log;
+ * no message is lost or written to a destroyed FILE *.
+ * @note When diagnostic output is disabled or redirected to a non-file
+ * sink (e.g., syslog), diags_log is null or uninitialized and the
+ * initialization guard causes an early false return.
+ */
bool reseat_diagslog();
+
+ /**
+ * @brief Roll diags.log if the current rolling policy condition is met.
+ *
+ * Under a size-based policy, the current file's size is compared to the
+ * configured threshold. Under a time-based policy, the elapsed time since
+ * the last roll is compared to the configured interval. Combined policies
+ * evaluate both conditions independently. When a condition is met the
+ * current file is flushed, renamed, and a fresh file is opened at the same
+ * path.
+ *
+ * @return True if the underlying file was renamed; false otherwise. True is
+ * returned even when reopening the fresh file at the original path fails.
+ * @pre None. A null or uninitialized diags_log is a safe no-op that
+ * returns false.
+ * @post On a successful roll the file at the configured path is a fresh,
+ * empty file and emission is directed to it. If renaming succeeded but
+ * reopening did not, the configured path is left absent and emission
+ * continues to the renamed file. With no roll, all state is unchanged.
+ * @par Errors
+ * None signaled. fstat (size-based policy only) and reopen failures
+ * silently suppress replacement; a completed rename is never reversed.
+ * @par Thread safety
+ * tag_table_lock is acquired only during the BaseLogFile
+ * pointer swap. The rolling-condition checks and file operations execute
+ * without a lock; the caller must ensure no concurrent reconfiguration
+ * (see config_roll_diagslog()).
+ */
bool should_roll_diagslog();
+
+ /**
+ * @brief Roll the standard-output redirection file if the current rolling
+ * policy condition is met.
+ *
+ * Under a size-based policy, the current file's size is compared to the
+ * configured threshold. Under a time-based policy, the elapsed time since
+ * the last roll is compared to the configured interval. Combined policies
+ * evaluate both conditions independently. When a condition is met both
+ * stdout_log and stderr_log are flushed, the underlying file is renamed,
+ * and a fresh file is opened at the same path; the process's stdout (and
+ * stderr, when redirected to the same path) are rebound to the new file.
+ * The time-based path also advances the last-roll timestamp.
+ *
+ * @return True if the underlying file was renamed; false otherwise.
+ * @pre stdout_log and stderr_log are non-null; violation aborts.
+ * When a roll occurs, stdout_log and stderr_log must refer to the same
+ * filesystem path; violation aborts.
Review Comment:
The `@pre` says precondition violations “abort”, but the implementation uses
ink_assert() (debug-only) rather than ink_release_assert(). In non-debug
builds, violating these preconditions is undefined behavior (likely a crash),
not a guaranteed abort.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]