Copilot commented on code in PR #13262:
URL: https://github.com/apache/trafficserver/pull/13262#discussion_r3408389228
##########
include/tscore/DiagsTypes.h:
##########
@@ -220,26 +559,277 @@ 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 Same as error(). For terminal levels, invokes cleanup_func (if
+ * set) then exits the process; does not return.
+ * @par Errors
+ * None signaled; I/O errors absorbed.
+ * @par Thread safety
+ * Safe to call concurrently.
+ */
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 taglist is null or a valid PCRE2 pattern string. The regex engine is
+ * PCRE2, not POSIX ERE.
Review Comment:
activate_taglist()’s `@pre` currently requires a valid PCRE2 pattern string,
but the implementation tolerates invalid patterns (it compiles and, on failure,
installs a regex that matches nothing). The precondition should not require
validity; the failure behavior is already described in @post.
##########
include/tscore/DiagsTypes.h:
##########
@@ -220,26 +559,277 @@ 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 Same as error(). For terminal levels, invokes cleanup_func (if
+ * set) then exits the process; does not return.
+ * @par Errors
+ * None signaled; I/O errors absorbed.
+ * @par Thread safety
+ * Safe to call concurrently.
+ */
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 taglist is null or a valid PCRE2 pattern string. 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).
+ * @param[in] ri Rolling interval in seconds (used by time-based policies).
+ * @param[in] rs Rolling size threshold in bytes (used by size-based
+ * policies).
+ * @pre None.
+ * @post Rolling policy fields are updated in the calling thread.
+ * @par Errors
+ * None.
+ * @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.
+ * @param[in] ri Rolling interval in seconds.
+ * @param[in] rs Rolling size threshold in bytes.
Review Comment:
The doc for config_roll_outputlog() says the rolling-size threshold
parameter is in bytes, but the implementation stores the value as MB (see
should_roll_outputlog(): outputlog_rolling_size is multiplied by BYTES_IN_MB).
Please update the parameter documentation to match the actual unit (MB).
##########
include/tscore/DiagsTypes.h:
##########
@@ -220,26 +559,277 @@ 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 Same as error(). For terminal levels, invokes cleanup_func (if
+ * set) then exits the process; does not return.
+ * @par Errors
+ * None signaled; I/O errors absorbed.
+ * @par Thread safety
+ * Safe to call concurrently.
+ */
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 taglist is null or a valid PCRE2 pattern string. 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).
+ * @param[in] ri Rolling interval in seconds (used by time-based policies).
+ * @param[in] rs Rolling size threshold in bytes (used by size-based
+ * policies).
Review Comment:
The doc for config_roll_diagslog() says the rolling-size threshold parameter
is in bytes, but the implementation stores the value as MB (see
should_roll_diagslog(): diagslog_rolling_size is multiplied by BYTES_IN_MB).
Please update the parameter documentation to match the actual unit (MB).
--
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]