From 2f86dd5a43baf22a6d27fd74f38da37aa28bbaf8 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 13 Apr 2018 16:52:24 +0200 Subject: erec: Review erec_print() A new requirement to erec for the upcoming JSON support is printing records with file input descriptors without open stream. The approach is to treat 'name' field as file name, open it, extract the offending line and close it again. Further changes to libnftables input parsing routines though have shown that the whole concept of file pointer reuse in erec is tedious and not worth keeping: * Closed files are to be supported as well, so there needs to be fallback code for opening the file anyway. * When input descriptor is duplicated from parser state into an error record, the file pointer is copied as well. Therefore care has to be taken to not free the parser state before any error records have been printed. This is the only point where old and duplicated input descriptors are connected. Therefore drop struct input_descriptor's 'fp' field and just always open the file by name. This way also the old stream offset doesn't have to be restored after reading. While being at it, this patch fixes two other (potential) problems: * If the offending line from input contains tabs, add them at the right position in the marker buffer as well to avoid misalignment. * The input file may not be seekable (/dev/stdin for instance), so skip printing of offending line and markers if it couldn't be read properly. Signed-off-by: Phil Sutter Signed-off-by: Pablo Neira Ayuso --- src/erec.c | 79 ++++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 43 insertions(+), 36 deletions(-) (limited to 'src/erec.c') diff --git a/src/erec.c b/src/erec.c index 226c51f5..617c04ad 100644 --- a/src/erec.c +++ b/src/erec.c @@ -118,16 +118,12 @@ void erec_print(struct output_ctx *octx, const struct error_record *erec, { const struct location *loc = erec->locations, *iloc; const struct input_descriptor *indesc = loc->indesc, *tmp; - const char *line = NULL; /* silence gcc */ + const char *line = NULL; char buf[1024] = {}; char *pbuf = NULL; unsigned int i, end; + FILE *f; int l; - off_t orig_offset = 0; - FILE *f = octx->error_fp; - - if (!f) - return; switch (indesc->type) { case INDESC_BUFFER: @@ -136,13 +132,16 @@ void erec_print(struct output_ctx *octx, const struct error_record *erec, *strchrnul(line, '\n') = '\0'; break; case INDESC_FILE: - orig_offset = ftell(indesc->fp); - if (orig_offset >= 0 && - !fseek(indesc->fp, loc->line_offset, SEEK_SET) && - fread(buf, 1, sizeof(buf) - 1, indesc->fp) > 0 && - !fseek(indesc->fp, orig_offset, SEEK_SET)) + f = fopen(indesc->name, "r"); + if (!f) + break; + + if (!fseek(f, loc->line_offset, SEEK_SET) && + fread(buf, 1, sizeof(buf) - 1, f) > 0) { *strchrnul(buf, '\n') = '\0'; - line = buf; + line = buf; + } + fclose(f); break; case INDESC_INTERNAL: case INDESC_NETLINK: @@ -151,6 +150,8 @@ void erec_print(struct output_ctx *octx, const struct error_record *erec, BUG("invalid input descriptor type %u\n", indesc->type); } + f = octx->error_fp; + if (indesc->type == INDESC_NETLINK) { fprintf(f, "%s: ", indesc->name); if (error_record_names[erec->type]) @@ -160,32 +161,34 @@ void erec_print(struct output_ctx *octx, const struct error_record *erec, loc = &erec->locations[l]; netlink_dump_expr(loc->nle, f, debug_mask); } - fprintf(f, "\n"); - } else { - if (indesc->location.indesc != NULL) { - const char *prefix = "In file included from"; - iloc = &indesc->location; - for (tmp = iloc->indesc; - tmp != NULL && tmp->type != INDESC_INTERNAL; - tmp = iloc->indesc) { - fprintf(f, "%s %s:%u:%u-%u:\n", prefix, - tmp->name, - iloc->first_line, iloc->first_column, - iloc->last_column); - prefix = " from"; - iloc = &tmp->location; - } + fprintf(f, "\n\n"); + return; + } + + if (indesc->location.indesc != NULL) { + const char *prefix = "In file included from"; + iloc = &indesc->location; + for (tmp = iloc->indesc; + tmp != NULL && tmp->type != INDESC_INTERNAL; + tmp = iloc->indesc) { + fprintf(f, "%s %s:%u:%u-%u:\n", prefix, + tmp->name, + iloc->first_line, iloc->first_column, + iloc->last_column); + prefix = " from"; + iloc = &tmp->location; } - if (indesc->name != NULL) - fprintf(f, "%s:%u:%u-%u: ", indesc->name, - loc->first_line, loc->first_column, - loc->last_column); - if (error_record_names[erec->type]) - fprintf(f, "%s: ", error_record_names[erec->type]); - fprintf(f, "%s\n", erec->msg); + } + if (indesc->name != NULL) + fprintf(f, "%s:%u:%u-%u: ", indesc->name, + loc->first_line, loc->first_column, + loc->last_column); + if (error_record_names[erec->type]) + fprintf(f, "%s: ", error_record_names[erec->type]); + fprintf(f, "%s\n", erec->msg); - if (indesc->type != INDESC_INTERNAL) - fprintf(f, "%s\n", line); + if (line) { + fprintf(f, "%s\n", line); end = 0; for (l = erec->num_locations - 1; l >= 0; l--) { @@ -194,6 +197,10 @@ void erec_print(struct output_ctx *octx, const struct error_record *erec, } pbuf = xmalloc(end + 1); memset(pbuf, ' ', end + 1); + for (i = 0; i < end && line[i]; i++) { + if (line[i] == '\t') + pbuf[i] = '\t'; + } for (l = erec->num_locations - 1; l >= 0; l--) { loc = &erec->locations[l]; for (i = loc->first_column ? loc->first_column - 1 : 0; -- cgit v1.2.3