fix(warnings): rename the browser-app locals that hide a parameter and a local by zackees · Pull Request #15 · obs-rust/obs-browser

zackees · 2026-10-28T03:27:42Z

obs-rust/obs-studio now compiles at MSVC /W4 with warnings-as-errors (obs-rust/obs-studio#2626). The Windows app build, which is the only lane that compiles this plugin, reports two findings in browser-app.cpp:

browser-app.cpp:447 C4457 frame hides the function parameter
browser-app.cpp:487 C4456 args hides a previous local in the same function

Both are shadowing only: the inner CefFrame loop variable becomes target_frame and the CefV8ValueList becomes callback_args, with every use in their own scope updated. No behavioural change, no signature change.

Verified with clang-format --style=file --dry-run --Werror browser-app.cpp (clean); this repo has no compiler lane, so the build check is the obs-studio /W4 lane after the pointer bump.

Refs obs-rust/obs-studio#2626

…d a local MSVC /W4 reports C4457 ("frame" hides the function parameter) and C4456 ("args" hides the previous local) in browser-app.cpp; both are shadowing only, so the inner names change and the outer ones keep their meaning. Refs obs-rust/obs-studio#2626.

zackees · 2026-10-28T03:34:09Z

Reviewed the diff and the surrounding function body (browser-app.cpp OnProcessMessageReceived), not only the hunks.

Correctness. frame → target_frame is behaviour-neutral: the loop at :444-467 only uses the value for GetV8Context(), and no other reference to frame existed in that scope after the rename. args → callback_args in the executeCallback branch also stays correct: the CefV8ValueList is still populated with the single parsed retval before ExecuteFunction, and the other arguments in that branch is the CefListValue argument list, so the two names no longer collide.

Why it is worth landing. These are exactly the C4458/C4456 shadowing diagnostics the /W4 ratchet reports in the browser app (obs-studio#2626, "browser-app locals that hide a parameter and a local"). The rename clears them without changing any call, and the only check this repo runs (Format 🔍 / clang-format) is green on the head.

Not verified here. The archive-side claim in obs-studio#2626 (that the app still compiles under MSVC /W4 after this) is proved by the next Windows app build, not by this repo. Approving on the mechanical diff and the format lane.

…2626) Run 40628508954 compiled past configure and reported the next class, all first-party: - aja-output-ui: three `OBSData data` locals hid QWidget::data (C4458) and three `cardManager` declarations hid the file-static global (C4459); inner names only. - graphics-hook/gl-capture: the blacklist lookup's `process_name` hid the global (C4459). - obs-ffmpeg-mpegts: `to_join` is assigned under `have_to_join` and only read under it, but MSVC cannot correlate the flag across the mutex unlock (C4701); it is now `= {0}`. - obs-nvenc: `enableLookahead` is a uint32_t assigned to a bool (C4244) - now `!= 0`, which is exactly what the implicit conversion did; `lookaheadDepth` gets the explicit uint16_t cast its NVENC field already required. Two findings in this wave are submodule code and are fixed there instead: obs-browser (obs-rust/obs-browser#15) and obs-websocket's onMessage (obs-rust/obs-websocket#24). The vendored librtmp class stays with the maintainer (part 1 of the escalation on #2626). Refs #2626.

…names (#2626) The app lane's remaining expected reds were the two submodule targets. Their fixes are merged in obs-rust/obs-browser#15 (C4457 frame, C4456 args) and obs-rust/obs-websocket#24 (C4456 errorCode x2); this pins each pointer at or above those commits so a later pointer move cannot silently reintroduce the findings.

obs-browser b9c5db13 (obs-rust/obs-browser#15) and obs-websocket 960b99e7 (obs-rust/obs-websocket#24) both descend from master's pointers, so these bumps are fast-forwards of the submodule history, not rebases of it.