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.

Sign up for GitHub

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

Jump to bottom

Add clearer warnings on missing report elements specified in fmt files #562

Merged
merged 2 commits into from
Jun 3, 2019
Merged

Add clearer warnings on missing report elements specified in fmt files #562

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
543f457
Add clearer warnings about missing report elements specified in forma…
michalfabik May 24, 2019
0ab15ca
Rework and refine composition of error messages
michalfabik May 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions src/lib/problem_report.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,13 +305,16 @@ load_stream(FILE *fp)

#define MAX_OPT_DEPTH 10
static int
format_percented_string(const char *str, problem_data_t *pd, FILE *result)
format_percented_string(const char *str, problem_data_t *pd, FILE *result, char *fmt_file)
{
long old_pos[MAX_OPT_DEPTH] = { 0 };
int okay[MAX_OPT_DEPTH] = { 1 };
long len = 0;
int opt_depth = 1;

char *missing_item = xstrdup(str);
michalfabik marked this conversation as resolved.
Show resolved Hide resolved
memset(missing_item, 0, strlen(str));

while (*str) {
switch (*str) {
default:
Expand Down Expand Up @@ -365,6 +368,18 @@ format_percented_string(const char *str, problem_data_t *pd, FILE *result)

*nextpercent = '\0';
const problem_item *item = problem_data_get_item_or_NULL(pd, str);
if (!item)
{
if (opt_depth > 1)
{
log_debug("Missing content element: '%s'", str);
}
if (opt_depth == 1)
{
log_debug("Missing top-level element: '%s'", str);
strcpy(missing_item, str);
}
}
*nextpercent = '%';

if (item && (item->flags & CD_FLAG_TXT))
Expand All @@ -386,8 +401,10 @@ format_percented_string(const char *str, problem_data_t *pd, FILE *result)

if (!okay[0])
{
error_msg("Undefined variable outside of [[ ]] bracket");
error_msg("In format file '%s':", fmt_file);
error_msg(" Undefined variable '%s' outside [[ ]] brackets", missing_item);
michalfabik marked this conversation as resolved.
Show resolved Hide resolved
}
free(missing_item);

return 0;
}
Expand Down Expand Up @@ -1036,6 +1053,7 @@ struct problem_formatter
GList *pf_extra_sections; ///< user configured sections (struct extra_section)
char *pf_default_summary; ///< default summary format
problem_report_settings_t pf_settings; ///< settings for report generating
char *fmt_file;
};

static problem_report_settings_t
Expand Down Expand Up @@ -1085,6 +1103,9 @@ problem_formatter_free(problem_formatter_t *self)
free(self->pf_default_summary);
self->pf_default_summary = DESTROYED_POINTER;

free(self->fmt_file);
self->fmt_file = DESTROYED_POINTER;

free(self);
}

Expand Down Expand Up @@ -1187,6 +1208,7 @@ problem_formatter_load_file(problem_formatter_t *self, const char *path)
}

self->pf_sections = load_stream(fp);
self->fmt_file = xstrdup(path);

if (fp != stdin)
fclose(fp);
Expand Down Expand Up @@ -1215,7 +1237,7 @@ problem_formatter_generate_report(const problem_formatter_t *self, problem_data_
{
has_summary = true;
format_percented_string((const char *)section->items->data, data,
problem_report_get_buffer(pr, PR_SEC_SUMMARY));
problem_report_get_buffer(pr, PR_SEC_SUMMARY), self->fmt_file);
}
/* %attach as well */
else if (strcmp(section->name, "%attach") == 0)
Expand All @@ -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);
Copy link
Collaborator

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 GErrors 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.

Copy link
Collaborator

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.

Copy link
Collaborator

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).

}

*report = pr;
Expand Down