make workspace load more robust

there were some issues with error handling

Author: jcupitt

src/compile.c

@@ -1758,6 +1758,9 @@ compile_check(Compile *compile)
 void
 compile_error_set(Compile *compile)
 {
+	g_assert(strlen(error_get_top()) > 0);
+	g_assert(strlen(error_get_sub()) > 0);
+
 	(void) slist_map(compile->exprs, (SListMapFn) expr_error_set, NULL);
 }
 

src/expr.c

@@ -257,11 +257,9 @@ expr_strip(Expr *expr)
 
 	/* Break top links we're part of.
 	 */
-	if (slist_map(expr->static_links,
-			(SListMapFn) link_expr_destroy, NULL))
+	if (slist_map(expr->static_links, (SListMapFn) link_expr_destroy, NULL))
 		return expr;
-	if (slist_map(expr->dynamic_links,
-			(SListMapFn) link_expr_destroy, NULL))
+	if (slist_map(expr->dynamic_links, (SListMapFn) link_expr_destroy, NULL))
 		return expr;
 	g_assert(!expr->static_links);
 	g_assert(!expr->dynamic_links);
@@ -460,21 +458,25 @@ expr_error_set(Expr *expr)
 #ifdef DEBUG_ERROR
 		printf("expr_error_set: error in ");
 		symbol_name_print(expr->sym);
-		printf(": %s %s\n", error_get_top(), error_get_sub());
+		printf(": top=\"%s\" sub=\"%s\"\n", error_get_top(), error_get_sub());
 #endif /*DEBUG_ERROR*/
 
+		g_assert(strlen(error_get_top()) > 0);
+		g_assert(strlen(error_get_sub()) > 0);
+
 		VIPS_SETSTR(expr->error_sub, error_get_sub());
 
+		char txt[MAX_LINELENGTH];
+		VipsBuf buf = VIPS_BUF_STATIC(txt);
+
 		// add " in A1" etc. to the top error
+		vips_buf_appends(&buf, error_get_top());
 		if (expr->row) {
-			char txt[MAX_LINELENGTH];
-			VipsBuf buf = VIPS_BUF_STATIC(txt);
-
+			vips_buf_appends(&buf, " in ");
 			row_qualified_name(expr->row, &buf);
-			error_top(_("%s in %s"), error_get_top(), vips_buf_all(&buf));
 		}
 
-		VIPS_SETSTR(expr->error_top, error_get_top());
+		VIPS_SETSTR(expr->error_top, vips_buf_all(&buf));
 
 		/* Zap the value of the expr ... it may contain pointers to
 		 * dead stuff.
@@ -525,12 +527,14 @@ expr_error_clear(Expr *expr)
 		printf("\n");
 #endif /*DEBUG_ERROR*/
 
-		expr->err = FALSE;
 		expr_error_all = g_slist_remove(expr_error_all, expr);
 		if (expr->row)
 			row_error_clear(expr->row);
 
-		if (is_top(expr->sym) && expr->sym->expr == expr)
+		expr->err = FALSE;
+
+		if (is_top(expr->sym) &&
+			expr->sym->expr == expr)
 			symbol_state_change(expr->sym);
 	}
 }

src/itext.c

@@ -125,10 +125,11 @@ itext_add_string(PElement *base, VipsBuf *buf)
 			return base;
 	}
 
-	/* Buffer full? Abort string print.
+	/* Buffer full? Abort string print, though it's not an error (return
+	 * NULL).
 	 */
 	if (buf->full)
-		return base;
+		return NULL;
 
 	return NULL;
 }
@@ -335,8 +336,7 @@ itext_add_element(VipsBuf *buf, PElement *base,
 		vips_buf_appends(buf, "'");
 	}
 	else if (PEISCOMPLEX(base)) {
-		itext_add_complex(PEGETREALPART(base), PEGETIMAGPART(base),
-			buf);
+		itext_add_complex(PEGETREALPART(base), PEGETIMAGPART(base), buf);
 	}
 	else if (PEISMANAGEDSTRING(base)) {
 		Managedstring *managedstring = PEGETMANAGEDSTRING(base);
@@ -596,10 +596,13 @@ itext_update_model(Heapmodel *heapmodel)
 
 	/* If this is a non-edited row, update the source.
 	 */
-	if (!itext->edited || row == row->top_row) {
+	if (!itext->edited ||
+		row == row->top_row) {
 		const char *new_formula;
 
-		if (expr && expr->compile && expr->compile->rhstext)
+		if (expr &&
+			expr->compile &&
+			expr->compile->rhstext)
 			new_formula = expr->compile->rhstext;
 		else
 			new_formula = vips_buf_all(&itext->decompile);

src/row.c

@@ -254,6 +254,8 @@ row_dirty_set(Row *row, gboolean clear_error)
 void
 row_error_set(Row *row)
 {
+	workspace_error_sanity(row->ws);
+
 	if (!row->err) {
 		Workspace *ws = row->ws;
 		gboolean was_clear = ws->errors == NULL;
@@ -295,6 +297,8 @@ row_error_set(Row *row)
 			expr_error_set(row->top_row->expr);
 		}
 	}
+
+	workspace_error_sanity(row->ws);
 }
 
 /* Clear error state ... called from expr_error_clear() ... don't call this
@@ -303,7 +307,9 @@ row_error_set(Row *row)
 void
 row_error_clear(Row *row)
 {
-	if (row->err && row->ws) {
+	workspace_error_sanity(row->ws);
+
+	if (row->err) {
 		Workspace *ws = row->ws;
 
 		ws->errors = g_slist_remove(ws->errors, row);
@@ -321,9 +327,9 @@ row_error_clear(Row *row)
 		 * The code may contain pointers to dead symbols if we were in
 		 * error because they were undefined.
 		 */
-		if (row->child_rhs && row->child_rhs->itext)
-			heapmodel_set_modified(
-				HEAPMODEL(row->child_rhs->itext), TRUE);
+		if (row->child_rhs &&
+			row->child_rhs->itext)
+			heapmodel_set_modified(HEAPMODEL(row->child_rhs->itext), TRUE);
 
 		/* All errors gone? Ws changed too.
 		 */
@@ -333,11 +339,14 @@ row_error_clear(Row *row)
 		/* Is this a local row? Clear the top row error as well, in
 		 * case it's in error because of us.
 		 */
-		if (row != row->top_row && row->top_row->expr) {
+		if (row != row->top_row &&
+			row->top_row->expr) {
 			expr_error_clear(row->top_row->expr);
 			row_dirty_set(row->top_row, TRUE);
 		}
 	}
+
+	workspace_error_sanity(row->ws);
 }
 
 /* Break a dependency.
@@ -387,9 +396,11 @@ row_dispose(GObject *gobject)
 
 	/* Reset state. Also see row_parent_remove().
 	 */
-	row_hide_dependents(row);
 	if (row->expr)
 		expr_error_clear(row->expr);
+
+	row_hide_dependents(row);
+
 	if (col &&
 		col->last_select == row)
 		col->last_select = NULL;
@@ -1571,6 +1582,8 @@ row_recomp_row(Row *row)
 static void
 row_recomp_all(Row *top_row)
 {
+	workspace_error_sanity(top_row->ws);
+
 	/* Rebuild all dirty rows.
 	 */
 	while (!top_row->err && top_row->recomp) {
@@ -1608,6 +1621,8 @@ row_recomp_all(Row *top_row)
 		pgraph(&top_row->expr->root);
 #endif /*DEBUG*/
 	}
+
+	workspace_error_sanity(top_row->ws);
 }
 
 static GSList *

src/workspace.c

@@ -1429,9 +1429,37 @@ workspace_selected_group(Workspace *ws)
 	return TRUE;
 }
 
+void
+workspace_error_sanity(Workspace *ws)
+{
+#ifdef DEBUG
+	if (ws->errors) {
+		g_assert(!ws->last_error || ws->last_error->err);
+		g_assert(!ws->last_error || g_slist_find(ws->errors, ws->last_error));
+
+		GSList *p;
+		for (p = ws->errors; p; p = p->next) {
+			Row *row = ROW(p->data);
+
+			g_assert(row->top_row);
+			g_assert(row->top_row->expr);
+
+			g_assert(row->err);
+			g_assert(row->expr);
+			g_assert(strlen(row->expr->error_top) > 0);
+			g_assert(strlen(row->expr->error_sub) > 0);
+		}
+	}
+	else
+		g_assert(ws->last_error == NULL);
+#endif /*DEBUG*/
+}
+
 static Row *
 workspace_test_error(Row *row, Workspace *ws, int *found)
 {
+	g_assert(row->err);
+
 	/* Found next?
 	 */
 	if (*found)
@@ -1452,11 +1480,14 @@ workspace_test_error(Row *row, Workspace *ws, int *found)
 gboolean
 workspace_next_error(Workspace *ws)
 {
+	workspace_error_sanity(ws);
+
 	int found;
 
 	if (!ws->errors) {
 		error_top(_("No errors in tab"));
-		error_sub("%s", _("there are no errors (that I can see) in this tab"));
+		error_sub("%s",
+			_("there are no errors (that I can see) in this tab"));
 		return FALSE;
 	}
 
@@ -1484,6 +1515,8 @@ workspace_next_error(Workspace *ws)
 	error_top("%s", ws->last_error->expr->error_top);
 	error_sub("%s", ws->last_error->expr->error_sub);
 
+	workspace_error_sanity(ws);
+
 	return TRUE;
 }
 

src/workspace.h

@@ -173,6 +173,8 @@ void workspace_selected_remove_yesno(Workspace *ws, GtkWindow *parent);
 gboolean workspace_selected_ungroup(Workspace *ws);
 gboolean workspace_selected_group(Workspace *ws);
 
+void workspace_error_sanity(Workspace *ws);
+
 gboolean workspace_next_error(Workspace *ws);
 
 void workspace_set_mode(Workspace *ws, WorkspaceMode mode);