Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce and default to "minimal" crash report mode #9900

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

osyoyu
Copy link
Contributor

@osyoyu osyoyu commented Feb 9, 2024

vm_dump.c Outdated
@@ -1107,6 1107,19 @@ rb_vm_bugreport(const void *ctx, FILE *errout)
}
}

// Print a "minimal" crash report when errout is a TTY
int full_crash_report = !(isatty(fileno(errout)));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
int full_crash_report = !(isatty(fileno(errout)));
bool full_crash_report = !(isatty(fileno(errout)));

vm_dump.c Outdated
Comment on lines 1115 to 1120
if (strcmp(ruby_full_crash_report_env, "0") == 0) {
full_crash_report = false;
}
else {
full_crash_report = true;
}
Copy link
Member

Choose a reason for hiding this comment

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

This makes RUBY_FULL_CRASH_REPORT= ruby crash.rb full dump.
I'd expect that empty value would disable the feature.

Suggested change
if (strcmp(ruby_full_crash_report_env, "0") == 0) {
full_crash_report = false;
}
else {
full_crash_report = true;
}
full_crash_report = strtoul(ruby_full_crash_report_env, 0, 0) != 0;

Or even:

Suggested change
if (strcmp(ruby_full_crash_report_env, "0") == 0) {
full_crash_report = false;
}
else {
full_crash_report = true;
}
bool full_crash_report = ruby_full_crash_report_env ?
strtoul(ruby_full_crash_report_env, 0, 0) != 0 :
!isatty(fileno(errout));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! I agree that RUBY_FULL_CRASH_REPORT= ruby crash.rb should disable the feature.
Fixed in 4a3ebd4.

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

Successfully merging this pull request may close these issues.

2 participants