Skip to content

gcode: guard NULL chained on_settings_changed in onSettingsChanged - #999

Open
ScottW514 wants to merge 1 commit into
grblHAL:masterfrom
ScottW514:fix-null-on-settings-changed
Open

gcode: guard NULL chained on_settings_changed in onSettingsChanged#999
ScottW514 wants to merge 1 commit into
grblHAL:masterfrom
ScottW514:fix-null-on-settings-changed

Conversation

@ScottW514

Copy link
Copy Markdown

gc_init() subscribes to grbl.on_settings_changed by capturing the previous handler so it can chain it:

if(settings_changed == NULL) {
    settings_changed = grbl.on_settings_changed;   // may legitimately be NULL
    grbl.on_settings_changed = onSettingsChanged;
}

When no plugin or driver has subscribed before the first gc_init() runs, the captured pointer is NULL, and the chained call at the top of onSettingsChanged() dereferences it. On builds with no other subscriber, every runtime $-setting write then crashes (observed live with the Simulator: any $xx=yy segfaulted).

The core dispatcher in grbllib.c already NULL-checks its own grbl.on_settings_changed before calling; this adds the same guard to the chained tail in gcode.c. No behavior change when a subscriber exists.

Reproduce (before this fix): build the Simulator, connect, send any $-setting write, e.g. $110=6000 - the process crashes in onSettingsChanged().

Comment thread gcode.c Outdated
gc_init subscribes to grbl.on_settings_changed by capturing the previous
handler to chain it. When no plugin or driver has subscribed before the
first gc_init, the captured pointer is NULL and the chained call in
onSettingsChanged dereferences it - every runtime $-setting write then
crashes on drivers with no other subscriber (observed live with the
Simulator). The core dispatcher already NULL-checks its own pointer;
this adds the same guard to the chained tail.
@ScottW514
ScottW514 force-pushed the fix-null-on-settings-changed branch from 6869c58 to 0f333a6 Compare August 3, 2026 10:05
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.

2 participants