Skip to content

fix: return default_value from get_env<int> on unparsable env values - #736

Open
shoemoney wants to merge 1 commit into
deepseek-ai:mainfrom
shoemoney:fix/get-env-uninitialized
Open

fix: return default_value from get_env<int> on unparsable env values#736
shoemoney wants to merge 1 commit into
deepseek-ai:mainfrom
shoemoney:fix/get-env-uninitialized

Conversation

@shoemoney

Copy link
Copy Markdown

get_env<int> (csrc/utils/system.hpp) declared int value; uninitialized and ignored sscanf's return value. Any env value %d cannot parse (true, false, on, off, empty string) returned indeterminate stack memory instead of falling back to default_value. In practice the garbage is usually non-zero, so EP_BUFFER_DEBUG=false reads as enabled. This helper backs 20+ env reads across the codebase.

The fix initializes value to default_value and returns default_value when sscanf does not report a successful conversion.

Verified with a standalone host build of the header: before the change, EP_X=false with default 7 returned 1 (uninitialized stack); after, it returns 7, EP_X=42 returns 42, and unset still returns the default.

Fixes #733

get_env<int> left `value` uninitialized and ignored sscanf's return, so a
non-numeric value (e.g. EP_BUFFER_DEBUG=false) returned uninitialized stack
memory instead of falling back to default_value.
Comment thread csrc/utils/system.hpp
int value;
std::sscanf(c_str, "%d", &value);
int value = default_value;
if (std::sscanf(c_str, "%d", &value) != 1)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 suggestion: 可选改进:sscanf("42abc", "%d") 仍会成功解析为 42,且 false/true 等布尔风格取值只是静默回退到默认值。若希望更严格,可考虑用 std::from_chars 校验整个字符串,或对无法解析的取值打印一次告警,帮助用户发现拼写错误的环境变量值。非阻塞。

🤖 v5

@ds-review-bot

Copy link
Copy Markdown
Collaborator

🤖 ds-review-bot Code Review

v6

变更正确检查了 sscanf 的转换结果,并在无法解析整数时返回默认值,修复了未初始化值问题,未发现新的功能性缺陷。

v5

该 MR 修复了 csrc/utils/system.hpp 中 get_env<int> 的未定义行为:原实现声明未初始化的 int value; 且忽略 sscanf 返回值,导致环境变量无法按 %d 解析时(如 falsetrueonoff、空字符串)返回不确定的栈内存值,实际中通常为非零,使 EP_BUFFER_DEBUG=false 等被错误当作启用。修复将 value 初始化为 default_value,并在 sscanf(...) != 1 时显式返回 default_value。改动最小、正确,覆盖了该辅助函数支撑的 20+ 处环境变量读取。已核对代码(commit 8686bd5):初始化与显式返回同时存在属合理的防御性写法;EP_X=false 现回退到默认值,EP_X=42 正常解析,未设置时返回默认值。建议合并。

v4p

本 MR 修复 get_env&lt;int&gt; 在环境变量无法被 %d 解析(如 true/false/on/off/空串)时返回未初始化栈内存、导致 default_value 失效的问题:将 value 初始化为默认值,并在 sscanf 未报告成功转换时返回默认值。整体评估:改动小而准确,正确处理了空串与不可解析输入,符合该 helper 的 fallback 契约,未发现需要阻塞合并的问题。

Files reviewed: 1
Issues found: 🔵 1 suggestion
Inline comments posted: 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

get_env<int> returns an uninitialized value when an env var is set to a non-numeric string (EP_BUFFER_DEBUG=false reads as true)

2 participants