On Mon, Apr 12, 2021 at 10:25:35AM -0600, Theo de Raadt wrote: > > + line = XML_GetCurrentLineNumber(p); > > I think you can simplify your larger diff and avoid the temporary variable > by doing: > > warnx("%s: XML error at line %llu: %s", s->local, > (unsigned long long)XML_GetCurrentLineNumber(p), > XML_ErrorString(XML_GetErrorCode(s->parser))); > > This is the explicit upcast model we use for a similar problem: time_t > > The benefit of doing the upcast directly in front of the function call > rather than defining a temporary variable with implicit upcast, became > clear during our time_t conversion process: we saw developers creating > implicit upcast temporary variables, and then passing the temporary time > variables (with a different type) to more than printf related code. > That felt wrong. We wanted to aovid breaking the non-64bit time_t use > cases of the code. Carrying time values in non-time_t types felt wrong, so > we preferred keeping the values in the correct type until the last moment, > hence (explicit cast)function() just for the "%llu".
In that case how about this? -- :wq Claudio Index: rrdp.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/rrdp.c,v retrieving revision 1.3 diff -u -p -r1.3 rrdp.c --- rrdp.c 7 Apr 2021 16:29:14 -0000 1.3 +++ rrdp.c 12 Apr 2021 16:50:05 -0000 @@ -303,18 +303,18 @@ rrdp_finished(struct rrdp *s) } if (s->res == HTTP_OK) { + XML_Parser p = s->parser; + /* * Finalize parsing on success to be sure that * all of the XML is correct. Needs to be done here * since the call would most probably fail for non * successful data fetches. */ - if (XML_Parse(s->parser, NULL, 0, 1) != XML_STATUS_OK) { - warnx("%s: XML error at line %lu: %s", - s->local, - XML_GetCurrentLineNumber(s->parser), - XML_ErrorString(XML_GetErrorCode(s->parser)) - ); + if (XML_Parse(p, NULL, 0, 1) != XML_STATUS_OK) { + warnx("%s: XML error at line %llu: %s", s->local, + (unsigned long long)XML_GetCurrentLineNumber(p), + XML_ErrorString(XML_GetErrorCode(p))); rrdp_failed(s); return; } @@ -340,16 +340,14 @@ rrdp_finished(struct rrdp *s) case SNAPSHOT: warnx("%s: downloading snapshot", s->local); - s->sxml = new_snapshot_xml(s->parser, - &s->current, s); + s->sxml = new_snapshot_xml(p, &s->current, s); s->state = RRDP_STATE_REQ; break; case DELTA: warnx("%s: downloading %lld deltas", s->local, s->repository.serial - s->current.serial); - s->dxml = new_delta_xml(s->parser, - &s->current, s); + s->dxml = new_delta_xml(p, &s->current, s); s->state = RRDP_STATE_REQ; break; } @@ -368,8 +366,7 @@ rrdp_finished(struct rrdp *s) } else { /* reset delta parser for next delta */ free_delta_xml(s->dxml); - s->dxml = new_delta_xml(s->parser, - &s->current, s); + s->dxml = new_delta_xml(p, &s->current, s); s->state = RRDP_STATE_REQ; } break; @@ -498,10 +495,10 @@ rrdp_data_handler(struct rrdp *s) SHA256_Update(&s->ctx, buf, len); if ((s->state & RRDP_STATE_PARSE_ERROR) == 0 && XML_Parse(p, buf, len, 0) != XML_STATUS_OK) { - s->state |= RRDP_STATE_PARSE_ERROR; - warnx("%s: parse error at line %lu: %s", s->local, - XML_GetCurrentLineNumber(p), + warnx("%s: parse error at line %llu: %s", s->local, + (unsigned long long)XML_GetCurrentLineNumber(p), XML_ErrorString(XML_GetErrorCode(p))); + s->state |= RRDP_STATE_PARSE_ERROR; } }