Skip to content

Remove try_break; its break never left the caller's loop - #168

Open
rlerdorf wants to merge 1 commit into
adsr:masterfrom
rlerdorf:remove-try-break
Open

rlerdorf wants to merge 1 commit into
adsr:masterfrom
rlerdorf:remove-try-break

Conversation

@rlerdorf

Copy link
Copy Markdown
Contributor

try_break in phpspy.h wraps if ((rv = call) != 0) break; in its own do { } while(0), so the break binds to the macro's loop rather than the caller's — it only ever assigned rv and fell through. Its single user is the -d epilogue block in event_handler_fout's STACK_END, which as a result kept trying to append # pid and the trace delimiter after # trace_ts had already failed to fit. Harmless in practice only because a full buffer made every subsequent attempt fail the same way.

This inlines the intended check so the do/while genuinely stops at the first record that doesn't fit, and removes the macro (nothing else references it). make is warning-free; test_verbose, test_pid, test_buffer_full, test_filter, test_flamegraph pass.

Found while working on #167, which deletes this whole block anyway. Whichever lands second: if #167 goes first this PR becomes a one-line phpspy.h change; if this goes first, #167's rewrite of STACK_END supersedes the inlined lines. Either conflict is trivial.

🤖 Generated with Claude Code

try_break wrapped `if ((rv = call) != 0) break;` in its own do { } while(0),
so the break bound to the macro's loop, not the caller's. It only ever
assigned rv and fell through. Its one user, the -d epilogue in
event_handler_fout's STACK_END, therefore kept trying to append the pid
field and the trace delimiter after the timestamp had already failed to
fit -- harmless only because a full buffer made every later attempt fail
the same way.

Inline the intended check so the do/while actually stops at the first
record that does not fit, and drop the macro since nothing else uses it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.

1 participant