From a68faceddf1dc86dc4b30b7669dda671f4f7ee08 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deuc=D0=B5?= <shurd@sasktel.net>
Date: Mon, 29 Jan 2024 23:11:31 -0500
Subject: [PATCH] This can now render the root and labels.

Still a bunch more work to do on labels... sizers come next.
---
 src/newifc/alltests.c        |  2 ++
 src/newifc/genapi.c          | 42 +++++++++++++++---------------
 src/newifc/internal_macros.h | 16 ++++++------
 src/newifc/label.c           | 21 ++++++---------
 src/newifc/root_window.c     | 50 +++++++++++++-----------------------
 src/xpdev/threadwrap.c       | 30 ++++++++++++++++++++++
 src/xpdev/threadwrap.h       |  1 +
 7 files changed, 89 insertions(+), 73 deletions(-)

diff --git a/src/newifc/alltests.c b/src/newifc/alltests.c
index b1ef0c1c38..ac5e26fe0c 100644
--- a/src/newifc/alltests.c
+++ b/src/newifc/alltests.c
@@ -4,6 +4,7 @@
 
 void test_root_window(CuTest *ct);
 void test_label(CuTest *ct);
+void test_api(CuTest *ct);
 
 void RunAllTests(void) {
 	CuString *output = CuStringNew();
@@ -11,6 +12,7 @@ void RunAllTests(void) {
 
 	SUITE_ADD_TEST(suite, test_root_window);
 	SUITE_ADD_TEST(suite, test_label);
+	SUITE_ADD_TEST(suite, test_api);
 
 	CuSuiteRun(suite);
 	CuSuiteSummary(suite, output);
diff --git a/src/newifc/genapi.c b/src/newifc/genapi.c
index 635eea4243..5160ffc1b4 100644
--- a/src/newifc/genapi.c
+++ b/src/newifc/genapi.c
@@ -80,7 +80,6 @@ attributes[] = {
 	{"height", NI_attr_type_uint16_t, attr_impl_global, 0, 0},
 	{"hidden", NI_attr_type_bool, attr_impl_global, 1, 0},
 	{"higherpeer", NI_attr_type_NewIfcObj, attr_impl_global, 1, 0},
-	{"last_error", NI_attr_type_NI_err, attr_impl_global, 1, 0},
 	{"locked", NI_attr_type_bool, attr_impl_root, 0, 1},
 	{"locked_by_me", NI_attr_type_bool, attr_impl_root, 1, 1},
 	{"lowerpeer", NI_attr_type_NewIfcObj, attr_impl_global, 1, 0},
@@ -162,11 +161,13 @@ attribute_functions(size_t i, FILE *c_code, const char *alias)
 						"	NI_err ret;\n"
 						"	if (obj == NULL)\n"
 						"		return NewIfc_error_invalid_arg;\n"
-						"	if (NI_set_locked(obj, true)) {\n", alias, type_str[attributes[i].type].type);
+						"	if (NI_set_locked(obj, true) == NewIfc_error_none) {\n", alias, type_str[attributes[i].type].type);
 				if (!attributes[i].no_event) {
 					fprintf(c_code, "		ret = call_%s_change_handlers(obj, NewIfc_%s, value);\n"
-							"		if (ret != NewIfc_error_none)\n"
-							"			return ret;\n", type_str[attributes[i].type].var_name, attributes[i].name);
+							"		if (ret != NewIfc_error_none) {\n"
+							"			NI_set_locked(obj, false);\n"
+							"			return ret;\n"
+							"		}\n", type_str[attributes[i].type].var_name, attributes[i].name);
 				}
 				fprintf(c_code, "		ret = obj->set(obj, NewIfc_%s, value);\n"
 						"		NI_set_locked(obj, false);\n"
@@ -184,7 +185,7 @@ attribute_functions(size_t i, FILE *c_code, const char *alias)
 					"		return NewIfc_error_invalid_arg;\n"
 					"	if (value == NULL)\n"
 					"		return NewIfc_error_invalid_arg;\n"
-					"	if (NI_set_locked(obj, true)) {\n"
+					"	if (NI_set_locked(obj, true) == NewIfc_error_none) {\n"
 					"		ret = obj->get(obj, NewIfc_%s, value);\n"
 					"		NI_set_locked(obj, false);\n"
 					"	}\n"
@@ -200,16 +201,18 @@ attribute_functions(size_t i, FILE *c_code, const char *alias)
 						"	NI_err ret;\n"
 						"	if (obj == NULL)\n"
 						"		return NewIfc_error_invalid_arg;\n"
-						"	if (NI_set_locked(obj, true)) {\n", alias, type_str[attributes[i].type].type);
+						"	if (NI_set_locked(obj, true) == NewIfc_error_none) {\n", alias, type_str[attributes[i].type].type);
 				if (!attributes[i].no_event) {
 					fprintf(c_code, "		ret = call_%s_change_handlers(obj, NewIfc_%s, value);\n"
-							"		if (ret != NewIfc_error_none)\n"
-							"			return ret;\n", type_str[attributes[i].type].var_name, attributes[i].name);
+							"		if (ret != NewIfc_error_none) {\n"
+							"			NI_set_locked(obj, false);\n"
+							"			return ret;\n"
+							"		}\n", type_str[attributes[i].type].var_name, attributes[i].name);
 				}
 				fprintf(c_code, "		ret = obj->set(obj, NewIfc_%s, value);\n"
-						"		if (ret != NewIfc_error_none && obj->last_error != NewIfc_error_not_implemented) {\n"
+						"		if (ret == NewIfc_error_not_implemented) {\n"
 						"			obj->%s = value;\n"
-						"			obj->last_error = NewIfc_error_none;\n"
+						"			ret = NewIfc_error_none;\n"
 						"		}\n"
 						"		NI_set_locked(obj, false);\n"
 						"	}\n"
@@ -228,11 +231,11 @@ attribute_functions(size_t i, FILE *c_code, const char *alias)
 					"		return NewIfc_error_invalid_arg;\n"
 					"	if (value == NULL)\n"
 					"		return NewIfc_error_invalid_arg;\n"
-					"	if (NI_set_locked(obj, true)) {\n"
+					"	if (NI_set_locked(obj, true) == NewIfc_error_none) {\n"
 					"		ret = obj->get(obj, NewIfc_%s, value);\n"
-					"		if ((ret != NewIfc_error_none) && obj->last_error != NewIfc_error_not_implemented) {\n"
+					"		if (ret == NewIfc_error_not_implemented) {\n"
 					"			*value = obj->%s;\n"
-					"			obj->last_error = NewIfc_error_none;\n"
+					"			ret = NewIfc_error_none;\n"
 					"		}\n"
 					"		NI_set_locked(obj, false);\n"
 					"	}\n"
@@ -247,8 +250,7 @@ attribute_functions(size_t i, FILE *c_code, const char *alias)
 						"NI_set_%s(NewIfcObj obj, %s value) {\n"
 						"	if (obj == NULL)\n"
 						"		return NewIfc_error_invalid_arg;\n"
-						"	NI_err ret = obj->root->set(obj, NewIfc_%s, value);\n"
-						"	obj->last_error = obj->root->last_error;\n"
+						"	NI_err ret = obj->root->set(obj->root, NewIfc_%s, value);\n"
 						"	return ret;\n"
 						"}\n\n", alias, type_str[attributes[i].type].type, attributes[i].name);
 			}
@@ -259,8 +261,7 @@ attribute_functions(size_t i, FILE *c_code, const char *alias)
 					"		return NewIfc_error_invalid_arg;\n"
 					"	if (value == NULL)\n"
 					"		return NewIfc_error_invalid_arg;\n"
-					"	NI_err ret = obj->root->get(obj, NewIfc_%s, value);\n"
-					"	obj->last_error = obj->root->last_error;\n"
+					"	NI_err ret = obj->root->get(obj->root, NewIfc_%s, value);\n"
 					"	return ret;\n"
 					"}\n\n", alias, type_str[attributes[i].type].type, attributes[i].name);
 			break;
@@ -272,10 +273,12 @@ attribute_functions(size_t i, FILE *c_code, const char *alias)
 				"	NI_err ret;\n"
 				"	if (obj == NULL || handler == NULL)\n"
 				"		return NewIfc_error_invalid_arg;\n"
-				"	if (NI_set_locked(obj, true)) {\n"
+				"	if (NI_set_locked(obj, true) == NewIfc_error_none) {\n"
 				"		struct NewIfc_handler *h = malloc(sizeof(struct NewIfc_handler));\n"
-				"		if (h == NULL)\n"
+				"		if (h == NULL) {\n"
+				"			NI_set_locked(obj, false);\n"
 				"			return NewIfc_error_allocation_failure;\n"
+				"		}\n"
 				"		h->on_%s_change = handler;\n"
 				"		h->cbdata = cbdata;\n"
 				"		h->event = NewIfc_on_%s_change;\n"
@@ -448,7 +451,6 @@ main(int argc, char **argv)
 	      "	uint32_t fg_colour;\n"
 	      "	uint32_t bg_colour;\n"
 	      "	enum NewIfc_object type;\n"
-	      "	NI_err last_error;\n"
 	      "	uint16_t height;\n"
 	      "	uint16_t width;\n"
 	      "	uint16_t min_height;\n"
diff --git a/src/newifc/internal_macros.h b/src/newifc/internal_macros.h
index aa9481babf..336f547b65 100644
--- a/src/newifc/internal_macros.h
+++ b/src/newifc/internal_macros.h
@@ -17,14 +17,14 @@
 	st->field = va_arg(ap, uint16_t); \
 } while(0);
 
-#define SET_STRING(st, field) do {                                 \
-	buf = strdup(va_arg(ap, const char *));                     \
-	if (buf == NULL) {                                           \
-		st->api.last_error = NewIfc_error_allocation_failure; \
-		break;                                                 \
-	}                                                               \
-	free(st->field);                                                 \
-	st->field = buf;                                                  \
+#define SET_STRING(st, field) do {                  \
+	buf = strdup(va_arg(ap, const char *));      \
+	if (buf == NULL) {                            \
+		ret = NewIfc_error_allocation_failure; \
+		break;                                  \
+	}                                                \
+	free(st->field);                                  \
+	st->field = buf;                                   \
 } while(0)
 
 
diff --git a/src/newifc/label.c b/src/newifc/label.c
index a6c17b29b3..3ea7ee8f29 100644
--- a/src/newifc/label.c
+++ b/src/newifc/label.c
@@ -26,41 +26,41 @@ static NI_err
 label_set(NewIfcObj obj, int attr, ...)
 {
 	struct label *l = (struct label *)obj;
+	NI_err ret = NewIfc_error_none;
 	SET_VARS;
 
-	l->api.last_error = NewIfc_error_none;
 	va_start(ap, attr);
 	switch (attr) {
 		case NewIfc_text:
 			SET_STRING(l, text);
 			break;
 		default:
-			l->api.last_error = NewIfc_error_not_implemented;
+			ret = NewIfc_error_not_implemented;
 			break;
 	}
 	va_end(ap);
 
-	return l->api.last_error;
+	return ret;
 }
 
 static NI_err
 label_get(NewIfcObj obj, int attr, ...)
 {
 	struct label *l = (struct label *)obj;
+	NI_err ret = NewIfc_error_none;
 	GET_VARS;
 
-	l->api.last_error = NewIfc_error_none;
 	va_start(ap, attr);
 	switch (attr) {
 		case NewIfc_text:
 			GET_STRING(l, text);
 			break;
 		default:
-			l->api.last_error = NewIfc_error_not_implemented;
+			ret = NewIfc_error_not_implemented;
 			break;
 	}
 
-	return l->api.last_error;
+	return ret;
 }
 
 static NI_err
@@ -125,7 +125,6 @@ NewIFC_label(NewIfcObj parent, NewIfcObj *newobj)
 	(*newl)->api.set = &label_set;
 	(*newl)->api.copy = &label_copy;
 	(*newl)->api.do_render = &label_do_render;
-	(*newl)->api.last_error = NewIfc_error_none;
 	(*newl)->api.child_ypos = 0;
 	(*newl)->api.child_xpos = 0;
 	(*newl)->api.child_width = 0;
@@ -154,27 +153,23 @@ void test_label(CuTest *ct)
 	struct vmem_cell cells;
 
 	CuAssertTrue(ct, NewIFC_root_window(NULL, &robj) == NewIfc_error_none);
+	CuAssertPtrNotNull(ct, robj);
 	CuAssertTrue(ct, NewIFC_label(robj, &obj) == NewIfc_error_none);
 	CuAssertPtrNotNull(ct, obj);
 	CuAssertPtrNotNull(ct, obj->get);
 	CuAssertPtrNotNull(ct, obj->set);
 	CuAssertPtrNotNull(ct, obj->copy);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
+	CuAssertPtrNotNull(ct, obj->do_render);
 	CuAssertTrue(ct, obj->width == 0);
 	CuAssertTrue(ct, obj->height == 1);
 	CuAssertTrue(ct, obj->min_width == 0);
 	CuAssertTrue(ct, obj->min_height == 1);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
 	CuAssertTrue(ct, obj->focus == true);
 	CuAssertTrue(ct, obj->get(obj, NewIfc_text, &s) == NewIfc_error_none && strcmp(s, "") == 0);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
 
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
 	CuAssertTrue(ct, obj->set(obj, NewIfc_text, new_title) == NewIfc_error_none);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
 
 	CuAssertTrue(ct, obj->get(obj, NewIfc_text, &s) == NewIfc_error_none && strcmp(s, new_title) == 0);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
 
 	CuAssertTrue(ct, robj->do_render(robj, NULL) == NewIfc_error_none);
 	size_t sz = strlen(new_title);
diff --git a/src/newifc/root_window.c b/src/newifc/root_window.c
index 45717554d5..5ffe0767a7 100644
--- a/src/newifc/root_window.c
+++ b/src/newifc/root_window.c
@@ -35,26 +35,26 @@ static NI_err
 rw_set(NewIfcObj obj, int attr, ...)
 {
 	struct root_window *rw = (struct root_window *)obj;
+	NI_err ret = NewIfc_error_none;
 	SET_VARS;
 
-	rw->api.last_error = NewIfc_error_none;
 	va_start(ap, attr);
 	switch (attr) {
 		case NewIfc_locked:
 			if (va_arg(ap, int)) {
 				if (pthread_mutex_lock(&rw->mtx) != 0) {
-					rw->api.last_error = NewIfc_error_lock_failed;
+					ret = NewIfc_error_lock_failed;
 					break;
 				}
 				rw->locks++;
 			}
 			else {
 				if (pthread_mutex_trylock(&rw->mtx) != 0) {
-					rw->api.last_error = NewIfc_error_lock_failed;
+					ret = NewIfc_error_lock_failed;
 					break;
 				}
 				if (rw->locks == 0) {
-					rw->api.last_error = NewIfc_error_lock_failed;
+					ret = NewIfc_error_lock_failed;
 					// TODO: Unlock failure can't be recovered and is silent... :(
 #ifndef NDEBUG
 					int lkret =
@@ -66,31 +66,31 @@ rw_set(NewIfcObj obj, int attr, ...)
 					break;
 				}
 				if (pthread_mutex_unlock(&rw->mtx) != 0) {
-					rw->api.last_error = NewIfc_error_lock_failed;
+					ret = NewIfc_error_lock_failed;
 				}
 				rw->locks--;
 				if (pthread_mutex_unlock(&rw->mtx) != 0) {
-					rw->api.last_error = NewIfc_error_lock_failed;
+					ret = NewIfc_error_lock_failed;
 				}
 			}
 			break;
 		default:
-			rw->api.last_error = NewIfc_error_not_implemented;
+			ret = NewIfc_error_not_implemented;
 			break;
 	}
 	va_end(ap);
 
-	return rw->api.last_error;
+	return ret;
 }
 
 static NI_err
 rw_get(NewIfcObj obj, int attr, ...)
 {
 	struct root_window *rw = (struct root_window *)obj;
+	NI_err ret = NewIfc_error_none;
 	GET_VARS;
 	int lkret;
 
-	rw->api.last_error = NewIfc_error_none;
 	va_start(ap, attr);
 	switch (attr) {
 		case NewIfc_locked:
@@ -99,16 +99,15 @@ rw_get(NewIfcObj obj, int attr, ...)
 			else {
 				*(va_arg(ap, bool *)) = rw->locks > 0;
 				if (pthread_mutex_unlock(&rw->mtx) != 0) {
-					rw->api.last_error = NewIfc_error_lock_failed;
-					assert(rw->api.last_error != NewIfc_error_lock_failed);
-					break;
+					ret = NewIfc_error_lock_failed;
+					assert(ret != NewIfc_error_lock_failed);
 				}
 			}
 			break;
 		case NewIfc_locked_by_me:
 			if ((lkret = pthread_mutex_trylock(&rw->mtx)) != 0) {
 				if (lkret != EBUSY) {
-					rw->api.last_error = NewIfc_error_lock_failed;
+					ret = NewIfc_error_lock_failed;
 					break;
 				}
 				*(va_arg(ap, bool *)) = false;
@@ -116,17 +115,17 @@ rw_get(NewIfcObj obj, int attr, ...)
 			else {
 				*(va_arg(ap, bool *)) = rw->locks > 0;
 				if (pthread_mutex_unlock(&rw->mtx) != 0) {
-					rw->api.last_error = NewIfc_error_lock_failed;
-					assert(rw->api.last_error != NewIfc_error_lock_failed);
+					ret = NewIfc_error_lock_failed;
+					assert(ret != NewIfc_error_lock_failed);
 				}
 			}
 			break;
 		default:
-			rw->api.last_error = NewIfc_error_not_implemented;
+			ret = NewIfc_error_not_implemented;
 			break;
 	}
 
-	return rw->api.last_error;
+	return ret;
 }
 
 static NI_err
@@ -185,6 +184,7 @@ rw_do_render_recurse(NewIfcObj obj, struct NewIfc_render_context *ctx)
 					if (ciolib_checkfont(obj->fill_font))
 						ctx->vmem[c].font = obj->fill_font;
 				}
+				c++;
 			}
 		}
 	}
@@ -256,8 +256,6 @@ NewIFC_root_window(NewIfcObj parent, NewIfcObj *newobj)
 	(*newrw)->api.get = rw_get;
 	(*newrw)->api.set = rw_set;
 	(*newrw)->api.copy = rw_copy;
-	(*newrw)->api.last_error = NewIfc_error_none;
-	(*newrw)->api.child_ypos = 1;
 	// TODO: This is only needed by the unit tests...
 	(*newrw)->api.root = *newobj;
 	(*newrw)->api.focus = true;
@@ -306,39 +304,27 @@ void test_root_window(CuTest *ct)
 	CuAssertPtrNotNull(ct, obj->get);
 	CuAssertPtrNotNull(ct, obj->set);
 	CuAssertPtrNotNull(ct, obj->copy);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
+	CuAssertPtrNotNull(ct, obj->do_render);
 	CuAssertTrue(ct, obj->width == 80);
 	CuAssertTrue(ct, obj->height == 25);
 	CuAssertTrue(ct, obj->min_width == 0);
 	CuAssertTrue(ct, obj->min_height == 0);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
 	CuAssertTrue(ct, obj->focus == true);
 	CuAssertTrue(ct, obj->get(obj, NewIfc_locked, &b) == NewIfc_error_none && !b);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
 	CuAssertTrue(ct, obj->get(obj, NewIfc_locked_by_me, &b) == NewIfc_error_none && !b);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
 	CuAssertTrue(ct, obj->fg_colour == NI_LIGHTGRAY);
 	CuAssertTrue(ct, obj->bg_colour == NI_BLACK);
 	CuAssertTrue(ct, obj->font == 0);
 
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
 	CuAssertTrue(ct, obj->set(obj, NewIfc_locked, false) == NewIfc_error_lock_failed);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_lock_failed);
 	CuAssertTrue(ct, obj->set(obj, NewIfc_locked, true) == NewIfc_error_none);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
 	CuAssertTrue(ct, obj->set(obj, NewIfc_locked_by_me, &b) == NewIfc_error_not_implemented);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_not_implemented);
 
 	CuAssertTrue(ct, obj->get(obj, NewIfc_locked, &b) == NewIfc_error_none && b);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
 	CuAssertTrue(ct, obj->get(obj, NewIfc_locked_by_me, &b) == NewIfc_error_none && b);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
 	CuAssertTrue(ct, obj->set(obj, NewIfc_locked, false) == NewIfc_error_none);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
 	CuAssertTrue(ct, obj->set(obj, NewIfc_locked, false) == NewIfc_error_lock_failed);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_lock_failed);
 	CuAssertTrue(ct, obj->get(obj, NewIfc_locked_by_me, &b) == NewIfc_error_none && !b);
-	CuAssertTrue(ct, obj->last_error == NewIfc_error_none);
 
 	CuAssertTrue(ct, obj->do_render(obj, NULL) == NewIfc_error_none);
 }
diff --git a/src/xpdev/threadwrap.c b/src/xpdev/threadwrap.c
index 18ed48b5d6..ed9f1a9521 100644
--- a/src/xpdev/threadwrap.c
+++ b/src/xpdev/threadwrap.c
@@ -86,6 +86,36 @@ ulong _beginthread(void( *start_address )( void * )
 
 #endif	/* __unix__ */
 
+/****************************************************************************/
+/* Wrappers for POSIX thread (pthread) mutexes								*/
+/****************************************************************************/
+bool pthread_mutex_init_np(pthread_mutex_t *mutex, bool recursive)
+{
+#if defined(_POSIX_THREADS)
+	pthread_mutexattr_t attr;
+	if (pthread_mutexattr_init(&attr) != 0)
+		return false;
+	if(recursive)
+#if defined(__linux__) && defined(PTHREAD_MUTEX_RECURSIVE_NP) && !defined(__USE_UNIX98)
+		if (pthread_mutexattr_settype(&attr,PTHREAD_MUTEX_RECURSIVE_NP) != 0) {
+			pthread_mutexattr_destroy(&attr);
+			return false;
+		}
+#else
+		if (pthread_mutexattr_settype(&attr,PTHREAD_MUTEX_RECURSIVE) != 0) {
+			pthread_mutexattr_destroy(&attr);
+			return false;
+		}
+#endif
+	pthread_mutex_init(mutex, &attr);
+	pthread_mutexattr_destroy(&attr);
+#else	/* Assumes recursive (e.g. Windows) */
+	(void)recursive;
+	pthread_mutex_init(mutex,NULL);
+#endif
+	return(true);
+}
+
 /****************************************************************************/
 /* Wrappers for POSIX thread (pthread) mutexes								*/
 /****************************************************************************/
diff --git a/src/xpdev/threadwrap.h b/src/xpdev/threadwrap.h
index cdb6cb7a35..e14c37fb6d 100644
--- a/src/xpdev/threadwrap.h
+++ b/src/xpdev/threadwrap.h
@@ -94,6 +94,7 @@ extern "C" {
 /* Wrappers for POSIX thread (pthread) mutexes								*/
 /****************************************************************************/
 
+bool pthread_mutex_init_np(pthread_mutex_t *mtx, bool recursive);
 pthread_mutex_t pthread_mutex_initializer_np(bool recursive);
 
 #if defined(_POSIX_THREADS)
-- 
GitLab