-
Notifications
You must be signed in to change notification settings - Fork 41
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
Add clearer warnings on missing report elements specified in fmt files #562
Conversation
@@ -1241,7 +1263,7 @@ problem_formatter_generate_report(const problem_formatter_t *self, problem_data_ | |||
self->pf_default_summary); | |||
|
|||
format_percented_string(self->pf_default_summary, | |||
data, problem_report_get_buffer(pr, PR_SEC_SUMMARY)); | |||
data, problem_report_get_buffer(pr, PR_SEC_SUMMARY), self->fmt_file); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had an idea for this: what if we started using GError
s more liberally and just propagated to callers? They will surely all have a pointer to the path somewhere. The downside is that you will need to add error handling to all of them, but a cursory glance tells me that it might already be done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://developer.gnome.org/glib/stable/glib-Error-Reporting.html for documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though another concern is breaking ABI compatibility, but, in Fedora at least, no one seems to be using libreport meaningfully (aside from us).
Ah, didn’t notice before, but there is libreport/src/lib/problem_report.c Line 1185 in ba0ee64
fmt_file member is null, and we end up printing a null string, which is undefined behavior, but glibc is lenient in that regard.
|
This should make the immediate cause of #445 more obvious.
Of course, the reason why the files might be missing from the directories in /var/spool/abrt in the first place is a whole different problem.