Update fz_try/fz_catch to be fz_try/{fz_always}/fz_catch.

Add explanations of how to use the macros in fitz.h.

Also included are 2 different formulations, with different strengths/
weaknesses for reference. Will remove these shortly, but I want a
reference to them in git.

Workaround bug in Mac OS Lion gcc (clang works fine).

3 files changed, 196 insertions, 4 deletions

diff --git a/fitz/base_error.c b/fitz/base_error.c
index f6badff6..24e5f642 100644
--- a/fitz/base_error.c
+++ b/fitz/base_error.c
@@ -46,7 +46,7 @@ void fz_warn(fz_context *ctx, char *fmt, ...)
 static void throw(fz_error_context *ex)
 {
 	if (ex->top >= 0) {
-		longjmp(ex->stack[ex->top--].buffer, 1);
+		longjmp(ex->stack[ex->top].buffer, 1);
 	} else {
 		fprintf(stderr, "uncaught exception: %s\n", ex->message);
 		LOGE("uncaught exception: %s\n", ex->message);
diff --git a/fitz/fitz.h b/fitz/fitz.h
index a5695d56..7788ffd6 100644
--- a/fitz/fitz.h
+++ b/fitz/fitz.h
@@ -128,6 +128,7 @@ struct fz_error_context_s
 {
 	int top;
 	struct {
+		int code;
 		jmp_buf buffer;
 	} stack[256];
 	char message[256];
@@ -136,16 +137,206 @@ struct fz_error_context_s
 void fz_var_imp(void *);
 #define fz_var(var) fz_var_imp((void *)&(var))
 
+/* MuPDF uses an set of exception handling macros to simplify error return
+ * and cleanup. Conceptually, they work a lot like C++'s try/catch system,
+ * but do not require any special compiler support.
+ *
+ * The basic formulation is as follows:
+ *
+ * fz_try(ctx)
+ * {
+ *	// Try to perform a task. Never 'return', 'goto' or 'longjmp' out
+ *	// of here. 'break' may be used to safely exit (just) the try block
+ *	// scope.
+ * }
+ * fz_always(ctx)
+ * {
+ *	// Any code here is always executed, regardless of whether an
+ *	// exception was thrown within the try or not. Never 'return', 'goto'
+ *	// or longjmp out from here. 'break' may be used to safely exit (just)
+ *	// the always block scope.
+ * }
+ * fz_catch(ctx)
+ * {
+ *	// This code is called (after any always block) only if something
+ *	// within the fz_try block (including any functions it called) threw
+ *	// an exception. The code here is expected to handle the exception
+ *	// (maybe record/report the error, cleanup any stray state etc) and
+ *	// can then either exit the block, or pass on the exception to a
+ *	// higher level (enclosing) fz_try block (using fz_throw, or
+ *	// fz_rethrow).
+ * }
+ *
+ * The fz_always block is optional, and can safely be omitted.
+ *
+ * The macro based nature of this system has 3 main limitations:
+ *
+ * 1) Never return from within try (or 'goto' or longjmp out of it).
+ *    This upsets the internal housekeeping of the macros and will cause
+ *    problems later on. The code will detect such things happening, but
+ *    by then it is too late to give a helpful error report as to where the
+ *    original infraction occurred.
+ *
+ * 2) The fz_try(ctx) { ... } fz_always(ctx) { ... } fz_catch(ctx) { ... }
+ *    is not one atomic C statement. That is to say, if you do:
+ *
+ *        if (condition)
+ *            fz_try(ctx) { ... }
+ *            fz_catch(ctx) { ... }
+ *
+ *    then you will not get what you want. Use the following instead:
+ *
+ *        if (condition) {
+ *            fz_try(ctx) { ... }
+ *            fz_catch(ctx) { ... }
+ *        }
+ *
+ * 3) The macros are implemented using setjmp and longjmp, and so the standard
+ *    C restrictions on the use of those functions apply to fz_try/fa_catch
+ *    too. In particular, any "truly local" variable that is set between the
+ *    start of fz_try and something in fz_try throwing an exception may become
+ *    undefined as part of the process of throwing that exception.
+ *
+ *    As a way of mitigating this problem, we provide an fz_var() macro that
+ *    tells the compiler to ensure that that variable is not unset by the
+ *    act of throwing the exception.
+ *
+ * A model piece of code using these macros then might be:
+ *
+ * house build_house(plans *p)
+ * {
+ *     material m = NULL;
+ *     walls w = NULL;
+ *     roof r = NULL;
+ *     house h = NULL;
+ *     tiles t = make_tiles();
+ *
+ *     fz_var(w);
+ *     fz_var(r);
+ *     fz_var(h);
+ *
+ *     fz_try(ctx)
+ *     {
+ *         fz_try(ctx)
+ *         {
+ *             m = make_bricks();
+ *         }
+ *         fz_catch(ctx)
+ *         {
+ *             // No bricks available, make do with straw?
+ *             m = make_straw();
+ *         }
+ *         w = make_walls(m, p);
+ *         r = make_roof(m, t);
+ *         h = combine(w, r);              // Note, NOT: return combine(w,r);
+ *     }
+ *     fz_always(ctx)
+ *     {
+ *         drop_walls(w);
+ *         drop_roof(r);
+ *         drop_material(m);
+ *         drop_tiles(t);
+ *     }
+ *     fz_catch(ctx)
+ *     {
+ *         fz_throw(ctx, "build_house failed");
+ *     }
+ *     return h;
+ * }
+ *
+ * Things to note about this:
+ *
+ *   a) If make_tiles throws an exception, this will immediately be handled
+ *      by some higher level exception handler. If it succeeds, t will be
+ *      set before fz_try starts, so there is no need to fz_var(t);
+ *
+ *   b) We try first off to make some bricks as our building material. If
+ *      this fails, we fall back to straw. If this fails, we'll end up in
+ *      the fz_catch, and the process will fail neatly.
+ *
+ *   c) We assume in this code that combine takes new reference to both the
+ *      walls and the roof it uses, and therefore that w and r need to be
+ *      cleaned up in all cases.
+ *
+ *   d) We assume the standard C convention that it is safe to destroy
+ *      NULL things.
+ */
+
+/* Exception macro definitions. Just treat these as a black box - pay no
+ * attention to the man behind the curtain. */
 #define fz_try(ctx) \
 	if (fz_push_try(ctx->error), \
-		!setjmp(ctx->error->stack[ctx->error->top].buffer)) \
+		(ctx->error->stack[ctx->error->top].code = setjmp(ctx->error->stack[ctx->error->top].buffer)) == 0) \
 	{ do {
 
+#define fz_always(ctx) \
+		} while (0); \
+	} \
+	{ do { \
+
 #define fz_catch(ctx) \
 		} while(0); \
-		ctx->error->top--; \
 	} \
-	else
+	if (ctx->error->stack[ctx->error->top--].code)
+
+/*
+ * We also include a couple of other formulations of the macros, with
+ * different strengths and weaknesses. These will be removed shortly, but
+ * I want them in git for at least 1 revision so I have a record of them.
+ *
+ * A formulation of try/always/catch that lifts limitation 2 above, but
+ * has problems when try/catch are nested in the same function; the inner
+ * nestings need to use fz_always_(ctx, label) and fz_catch_(ctx, label)
+ * instead. This was held as too high a price to pay to drop limitation 2.
+ *
+ * #define fz_try(ctx) \
+ *	if (fz_push_try(ctx->error), \
+ *		(ctx->error->stack[ctx->error->top].code = setjmp(ctx->error->stack[ctx->error->top].buffer)) == 0) \
+ *	{ do {
+ *
+ * #define fz_always_(ctx, label) \
+ *		} while (0); \
+ *		goto ALWAYS_LABEL_ ## label ; \
+ *	} \
+ *	else if (ctx->error->stack[ctx->error->top].code) \
+ *	{ ALWAYS_LABEL_ ## label : \
+ *		do {
+ *
+ * #define fz_catch_(ctx, label) \
+ *		} while(0); \
+ *		if (ctx->error->stack[ctx->error->top--].code) \
+ *			goto CATCH_LABEL_ ## label; \
+ *	} \
+ *	else if (ctx->error->top--, 1) \
+ *	CATCH_LABEL ## label:
+ *
+ * #define fz_always(ctx) fz_always_(ctx, TOP)
+ * #define fz_catch(ctx) fz_catch_(ctx, TOP)
+ *
+ * Another alternative formulation, that again removes limitation 2, but at
+ * the cost of an always block always costing us 1 extra longjmp per
+ * execution. Again this was felt to be too high a cost to use.
+ *
+ * #define fz_try(ctx) \
+ *	if (fz_push_try(ctx->error), \
+ *		(ctx->error->stack[ctx->error->top].code = setjmp(ctx->error->stack[ctx->error->top].buffer)) == 0) \
+ *	{ do {
+ *
+ * #define fz_always(ctx) \
+ *		} while (0); \
+ *		longjmp(ctx->error->stack[ctx->error->top].buffer, 3); \
+ *	} \
+ *	else if (ctx->error->stack[ctx->error->top].code & 1) \
+ *	{ do {
+ *
+ * #define fz_catch(ctx) \
+ *		} while(0); \
+ *		if (ctx->error->stack[ctx->error->top].code == 1) \
+ *			longjmp(ctx->error->stack[ctx->error->top].buffer, 2); \
+ *		ctx->error->top--;\
+ *	} \
+ *	else if (ctx->error->top--, 1)
+ */
 
 void fz_push_try(fz_error_context *ex);
 void fz_throw(fz_context *, char *, ...) __printflike(2, 3);
diff --git a/pdf/pdf_page.c b/pdf/pdf_page.c
index e3bed190..81e89e7f 100644
--- a/pdf/pdf_page.c
+++ b/pdf/pdf_page.c
@@ -241,6 +241,7 @@ pdf_load_page_contents_array(pdf_xref *xref, fz_obj *list)
 	big = fz_new_buffer(ctx, 32 * 1024);
 
 	n = fz_array_len(list);
+	fz_var(i); /* Workaround Mac compiler bug */
 	for (i = 0; i < n; i++)
 	{
 		fz_obj *stm = fz_array_get(list, i);