commit 91473d5f1fa4f5e83430127f82ec336551db3f28
from: Sven M. Hallberg <pesco@khjk.org>
date: Tue Mar 28 17:44:29 2023 UTC

restore single exit point in parse_xrefs

It turns out that this function was in fact meant to always assign a
result (NULL/0 on failure), accomplished by having a single exit point.
This was changed in 517b81ad for no reason. Reverting.

I'm guessing the goto was considered disagreeable, so I'll explain the
rationale. The function accumulates its result in the *local* variables
xrefs and n. This mainly makes the code nicer to read than writing to
the output directly. Having a single exit point, a property that is easy
to verify, ensures that no update to the local variables can get lost,
i.e. they serve as de-facto aliases for the outputs.

commit - f844fce33a0bdff13cb94ca9ab463ae1ef33bc3b
commit + 91473d5f1fa4f5e83430127f82ec336551db3f28
blob - a193e66c4b8bb5b1665f3524ceeef5cba081f9f2
blob + 3b282184f21c92d3d3c6b73983f07c53cb54b3bf
--- pdf.c
+++ pdf.c
@@ -4909,7 +4909,7 @@ parse_xrefs(struct Env *aux)
 	}
 	if (res == NULL) {
 		log_message(5, "VIOLATION[5]: startxref not found\n");
-		return;
+		goto end;
 	}
 	offset = H_INDEX_UINT(res->ast, 0);
 
@@ -4921,7 +4921,7 @@ parse_xrefs(struct Env *aux)
 	if ( (offset <=0) || (offset >= aux->sz) ) {
 		log_message(5, "VIOLATION[5]: Invalid xref table offset = %ld. Valid range <0, %ld>\n",
 				offset, aux->sz);
-		return;
+		goto end;
 	}
 
 	// XXX try formulating this loop as one parser using h_seek and h_bind
@@ -4976,6 +4976,7 @@ parse_xrefs(struct Env *aux)
 		offset = (size_t)tok->sint;
 	}
 
+end:
 	aux->xrefs = xrefs;
 	aux->nxrefs = n;
 }