ibus: die where startup fails; print the Plan 9 way; drop what said nothing

ibusinit unwound its server and un-registered its atexit handler on a
failure whose only sequel was ibusthread dying anyway; now each failure
dies with its own message, and the test that existed to walk that
unwind goes with it.  Also gone: an empty watch-toggle callback where
libdbus takes nil, Maxconns as a second name for Maxclients, fprintf
and strerror where the rest of the daemon says fprint and %r, USED()
where a parameter can simply be unnamed, a second findcontext() for the
Properties branch, and emitcommit() taking a connection and path apart
from the context that has both.  Ownerpoll lives once, in dat.h.

dat.h

@@ -25,6 +25,7 @@ enum
 enum
 {
 	Maxclients	= 64,
+	Ownerpoll	= 200,	/* ms between a frontend's owner checks while its preedit shows */
 	Maxkouho	= 128,
 	Maxdisp		= 9,
 

ibus.c

@@ -2,10 +2,7 @@
 #include "fn.h"
 
 #include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
 #include <errno.h>
-#include <unistd.h>
 #include <sys/stat.h>
 #include <poll.h>
 #include <dbus/dbus.h>
@@ -13,12 +10,10 @@
 
 enum
 {
-	Maxconns	= Maxclients,
-	Maxwatches	= 2*Maxconns + 1,
+	Maxwatches	= 2*Maxclients + 1,
 	/* Toolkits keep one context per widget that ever took focus. */
 	Maxcontexts	= 1024,
 	Relmask		= 1<<30,
-	Ownerpoll	= 200,	/* ms between owner checks while a preedit shows */
 	/* IBus wire constants; the daemon does not link libibus. */
 	Ibuscappreedit	= 1<<0,
 	Ibuspurposepassword = 8,
@@ -42,7 +37,7 @@ struct Ictx
 
 static DBusWatch *watches[Maxwatches];
 static int nwatches;
-static DBusConnection *conns[Maxconns];
+static DBusConnection *conns[Maxclients];
 static int nconns;
 static DBusServer *srv;
 static Ictx contexts[Maxcontexts];
@@ -70,10 +65,8 @@ unlinkaddr(void)
 }
 
 static int
-addrnote(void *v, char *note)
+addrnote(void*, char*)
 {
-	USED(v);
-	USED(note);
 	unlinkaddr();
 	return 0;
 }
@@ -211,11 +204,10 @@ writeaddr(char *path, char *addr)
 }
 
 static dbus_bool_t
-addwatch(DBusWatch *w, void *_)
+addwatch(DBusWatch *w, void*)
 {
 	int i;
 
-	USED(_);
 	for(i = 0; i < nwatches; i++)
 		if(watches[i] == nil){
 			watches[i] = w;
@@ -228,11 +220,10 @@ addwatch(DBusWatch *w, void *_)
 }
 
 static void
-removewatch(DBusWatch *w, void *_)
+removewatch(DBusWatch *w, void*)
 {
 	int i;
 
-	USED(_);
 	for(i = 0; i < nwatches; i++)
 		if(watches[i] == w){
 			watches[i] = nil;
@@ -240,13 +231,6 @@ removewatch(DBusWatch *w, void *_)
 		}
 }
 
-static void
-togglewatch(DBusWatch *w, void *_)
-{
-	USED(w);
-	USED(_);
-}
-
 static Ictx*
 findcontext(DBusConnection *conn, const char *path)
 {
@@ -385,15 +369,15 @@ newsignal(const char *path, const char *name)
 }
 
 static void
-emitcommit(DBusConnection *c, const char *path, const char *text)
+emitcommit(Ictx *ctx, const char *text)
 {
 	DBusMessage *sig;
 	DBusMessageIter it;
 
-	sig = newsignal(path, "CommitText");
+	sig = newsignal(ctx->path, "CommitText");
 	dbus_message_iter_init_append(sig, &it);
 	appendibustext(&it, text, 0);
-	dbus_connection_send(c, sig, nil);
+	dbus_connection_send(ctx->conn, sig, nil);
 	dbus_message_unref(sig);
 }
 
@@ -456,7 +440,7 @@ flushcontext(Ictx *ctx, int op)
 		emitpreedit(ctx, "");
 	stoutf(&res.commit, commit, sizeof commit);
 	if(commit[0] != '\0')
-		emitcommit(ctx->conn, ctx->path, commit);
+		emitcommit(ctx, commit);
 }
 
 /* Nobody is left to hand text to: the context or its connection is gone. */
@@ -619,7 +603,7 @@ handlekey(DBusConnection *c, DBusMessage *m, Ictx *ctx)
 	if(restart)
 		emitpreedit(ctx, "");
 	if(commit[0] != '\0')
-		emitcommit(c, dbus_message_get_path(m), commit);
+		emitcommit(ctx, commit);
 	if(clientpreedit(ctx) && (!restart || preedit[0] != '\0'))
 		emitpreedit(ctx, preedit);
 	return replybool(c, m, res.eaten);
@@ -861,11 +845,12 @@ onmsg(DBusConnection *c, DBusMessage *m, void*)
 			return handlecreate(c, m);
 	}
 	if(strcmp(iface, "org.freedesktop.DBus.Properties") == 0){
-		if(findcontext(c, path) == nil)
+		ctx = findcontext(c, path);
+		if(ctx == nil)
 			return handleerror(c, m, DBUS_ERROR_UNKNOWN_OBJECT,
 				"unknown input context");
 		if(strcmp(member, "Set") == 0)
-			return handlepropertyset(c, m, findcontext(c, path));
+			return handlepropertyset(c, m, ctx);
 		if(strcmp(member, "Get") == 0)
 			return handleerror(c, m, DBUS_ERROR_UNKNOWN_PROPERTY,
 				"input context properties are write-only");
@@ -903,28 +888,26 @@ onmsg(DBusConnection *c, DBusMessage *m, void*)
 }
 
 static void
-newconn(DBusServer *s, DBusConnection *c, void *_)
+newconn(DBusServer*, DBusConnection *c, void*)
 {
 	DBusObjectPathVTable vt;
 
-	USED(s);
-	USED(_);
-	if(nconns >= Maxconns){
-		fprintf(stderr, "strans: ibus: rejecting client: %d-connection limit reached\n",
-			Maxconns);
+	if(nconns >= Maxclients){
+		fprint(2, "strans: ibus: rejecting client: %d-connection limit reached\n",
+			Maxclients);
 		dbus_connection_close(c);
 		return;
 	}
 	if(!dbus_connection_set_watch_functions(c, addwatch, removewatch,
-	   togglewatch, nil, nil)){
-		fprintf(stderr, "strans: ibus: cannot watch client connection\n");
+	   nil, nil, nil)){
+		fprint(2, "strans: ibus: cannot watch client connection\n");
 		dbus_connection_close(c);
 		return;
 	}
 	memset(&vt, 0, sizeof(vt));
 	vt.message_function = onmsg;
 	if(!dbus_connection_register_fallback(c, "/", &vt, nil)){
-		fprintf(stderr, "strans: ibus: cannot register client handler\n");
+		fprint(2, "strans: ibus: cannot register client handler\n");
 		dbus_connection_set_watch_functions(c, nil, nil, nil, nil, nil);
 		dbus_connection_close(c);
 		return;
@@ -950,70 +933,35 @@ pruneconns(void)
 	nconns = j;
 }
 
-static int
+static void
 ibusinit(void)
 {
 	DBusError err;
-	char addr[128];
-	char *full;
-	int cleanupregistered;
+	char addr[64], *full;
 
-	full = nil;
-	cleanupregistered = 0;
-	if(buildaddrpath(addrfile, sizeof(addrfile)) < 0){
-		fprintf(stderr, "strans: ibus: cannot build address path\n");
-		return -1;
-	}
-	if(snprintf(addr, sizeof(addr), "unix:abstract=strans-%d",
-	   (int)getpid()) >= (int)sizeof addr){
-		fprintf(stderr, "strans: ibus: address is too long\n");
-		return -1;
-	}
+	if(buildaddrpath(addrfile, sizeof addrfile) < 0)
+		die("ibus: cannot build address path");
+	snprintf(addr, sizeof addr, "unix:abstract=strans-%d", (int)getpid());
 	dbus_error_init(&err);
 	srv = dbus_server_listen(addr, &err);
-	if(srv == nil){
-		fprintf(stderr, "strans: ibus: listen: %s\n", err.message);
-		dbus_error_free(&err);
-		addrfile[0] = '\0';
-		return -1;
-	}
+	if(srv == nil)
+		die("ibus: listen: %s", err.message);
 	dbus_server_set_new_connection_function(srv, newconn, nil, nil);
 	if(!dbus_server_set_watch_functions(srv, addwatch, removewatch,
-	   togglewatch, nil, nil)){
-		fprintf(stderr, "strans: ibus: cannot watch server\n");
-		goto fail;
-	}
+	   nil, nil, nil))
+		die("ibus: cannot watch server");
 	full = dbus_server_get_address(srv);
-	if(full == nil){
-		fprintf(stderr, "strans: ibus: cannot get server address\n");
-		goto fail;
-	}
-	if(!atexit(unlinkaddr)){
-		fprintf(stderr, "strans: ibus: cannot register address cleanup\n");
-		goto fail;
-	}
-	cleanupregistered = 1;
+	if(full == nil)
+		die("ibus: cannot get server address");
+	atexit(unlinkaddr);
 	threadnotify(addrnote, 1);
-	if(writeaddr(addrfile, full) < 0){
-		fprintf(stderr, "strans: ibus: cannot write %s\n", addrfile);
-		goto fail;
-	}
+	if(writeaddr(addrfile, full) < 0)
+		die("ibus: cannot write %s", addrfile);
 	dbus_free(full);
-	return 0;
-fail:
-	if(cleanupregistered)
-		atexitdont(unlinkaddr);
-	if(full != nil)
-		dbus_free(full);
-	dbus_server_disconnect(srv);
-	dbus_server_unref(srv);
-	srv = nil;
-	addrfile[0] = '\0';
-	return -1;
 }
 
 void
-ibusthread(void *_)
+ibusthread(void*)
 {
 	struct pollfd pfds[Maxwatches];
 	DBusWatch *polled[Maxwatches];
@@ -1021,13 +969,9 @@ ibusthread(void *_)
 	int i, n, rv;
 	unsigned int f;
 
-	USED(_);
 	threadsetname("ibus");
-	if(ibusinit() < 0)
-		die("ibus: initialization failed");
+	ibusinit();
 	replyc = chancreate(sizeof(Keyres), 0);
-	if(replyc == nil)
-		die("ibus: cannot create reply channel");
 	for(;;){
 		if(!dbus_server_get_is_connected(srv))
 			die("ibus: server disconnected");
@@ -1050,7 +994,7 @@ ibusthread(void *_)
 		if(rv < 0 && errno == EINTR)
 			continue;
 		if(rv < 0)
-			die("ibus: poll: %s", strerror(errno));
+			die("ibus: poll: %r");
 		for(i = 0; i < n; i++){
 			if(pfds[i].revents == 0)
 				continue;

tests/ibus_test.c

@@ -3,7 +3,6 @@
 
 #include <errno.h>
 
-
 enum
 {
 	Testctrlmask = 1<<2,
@@ -79,85 +78,6 @@ cleanup:
 	CT_CHECK(t, rmdir(root) == 0);
 }
 
-void
-ibus_startup_requires_ownership(struct ct *t)
-{
-	char root[] = "/tmp/strans-ibus-startup.XXXXXX";
-	char busdir[128], ibusdir[128], path[512], *old, *saved;
-	struct stat st;
-	int i, made, rv;
-
-	made = 0;
-	busdir[0] = '\0';
-	ibusdir[0] = '\0';
-	path[0] = '\0';
-	old = getenv("XDG_CONFIG_HOME");
-	saved = old == nil ? nil : strdup(old);
-	if(old != nil && !CT_CHECK(t, saved != nil))
-		return;
-	/* An IBus desktop names a live address file; the path must be ours. */
-	unsetenv("IBUS_ADDRESS_FILE");
-	if(!CT_CHECK(t, mkdtemp(root) != nil))
-		goto cleanup;
-	made = 1;
-	if(!CT_CHECK(t, snprintf(busdir, sizeof busdir, "%s/ibus/bus", root)
-	   < (int)sizeof busdir) ||
-	   !CT_CHECK(t, snprintf(ibusdir, sizeof ibusdir, "%s/ibus", root)
-	   < (int)sizeof ibusdir) ||
-	   !CT_CHECK(t, setenv("XDG_CONFIG_HOME", root, 1) == 0) ||
-	   !CT_CHECK(t, buildaddrpath(path, sizeof path) == 0))
-		goto cleanup;
-
-	memset(watches, 0, sizeof watches);
-	for(i = 0; i < nelem(watches); i++)
-		watches[i] = (DBusWatch*)1;
-	nwatches = nelem(watches);
-	rv = ibusinit();
-	CT_EQ_INT(t, -1, rv);
-	CT_EQ_PTR(t, nil, srv);
-	CT_CHECK(t, lstat(path, &st) < 0 && errno == ENOENT);
-	if(rv == 0){
-		dbus_server_disconnect(srv);
-		dbus_server_unref(srv);
-		srv = nil;
-		unlinkaddr();
-		atexitdont(unlinkaddr);
-	}
-
-	memset(watches, 0, sizeof watches);
-	nwatches = 0;
-	while(atexit(unlinkaddr))
-		continue;
-	CT_EQ_INT(t, -1, ibusinit());
-	CT_EQ_PTR(t, nil, srv);
-	CT_CHECK(t, lstat(path, &st) < 0 && errno == ENOENT);
-cleanup:
-	if(srv != nil){
-		dbus_server_disconnect(srv);
-		dbus_server_unref(srv);
-		srv = nil;
-	}
-	unlinkaddr();
-	atexitdont(unlinkaddr);
-	memset(watches, 0, sizeof watches);
-	nwatches = 0;
-	addrfile[0] = '\0';
-	if(saved != nil){
-		setenv("XDG_CONFIG_HOME", saved, 1);
-		free(saved);
-	}else
-		unsetenv("XDG_CONFIG_HOME");
-	if(made){
-		if(path[0] != '\0')
-			unlink(path);
-		if(busdir[0] != '\0')
-			CT_CHECK(t, rmdir(busdir) == 0 || errno == ENOENT);
-		if(ibusdir[0] != '\0')
-			CT_CHECK(t, rmdir(ibusdir) == 0 || errno == ENOENT);
-		CT_CHECK(t, rmdir(root) == 0);
-	}
-}
-
 static int
 ibusbegin(struct ct *t, Ibusfix *f)
 {

tests/test.h

@@ -108,7 +108,6 @@ void	server_connection_ownership(struct ct*);
 void	server_extension_stream(struct ct*);
 void	server_rejects_unknown_extension(struct ct*);
 void	ibus_machine_id_fallback(struct ct*);
-void	ibus_startup_requires_ownership(struct ct*);
 void	ibus_capability_policy(struct ct*);
 void	ibus_private_input_policy(struct ct*);
 void	ibus_context_lifecycle(struct ct*);

tests/unit_test.c

@@ -131,7 +131,6 @@ static const struct ct_test tests[] = {
 	{ "server/extension-stream", server_extension_stream },
 	{ "server/rejects-unknown-extension", server_rejects_unknown_extension },
 	{ "ibus/machine-id-fallback", ibus_machine_id_fallback },
-	{ "ibus/startup-requires-ownership", ibus_startup_requires_ownership },
 	{ "ibus/capability-policy", ibus_capability_policy },
 	{ "ibus/private-input-policy", ibus_private_input_policy },
 	{ "ibus/context-lifecycle", ibus_context_lifecycle },

xim/xim.c

@@ -32,11 +32,6 @@ struct Ic
 	Caret		caret;
 };
 
-enum
-{
-	Ownerpoll = 200,	/* ms between owner checks while a preedit shows */
-};
-
 static xcb_connection_t *conn;
 static xcb_im_t *xim;
 static struct xkb_state *kstate;