From cd637be4ca6e54b5b1ef5a09a531d2d0665ae09d Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Mon, 17 Aug 2026 01:34:06 +0900 Subject: [PATCH] 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 | 1 + ibus.c | 130 +++++++++++++--------------------------------- tests/ibus_test.c | 80 ---------------------------- tests/test.h | 1 - tests/unit_test.c | 1 - xim/xim.c | 5 -- 6 files changed, 38 insertions(+), 180 deletions(-) diff --git a/dat.h b/dat.h index 25b055e..e267ba3 100644 --- a/dat.h +++ b/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, diff --git a/ibus.c b/ibus.c index b443aff..1304bdb 100644 --- a/ibus.c +++ b/ibus.c @@ -2,10 +2,7 @@ #include "fn.h" #include -#include -#include #include -#include #include #include #include @@ -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; diff --git a/tests/ibus_test.c b/tests/ibus_test.c index f2b434c..c29df87 100644 --- a/tests/ibus_test.c +++ b/tests/ibus_test.c @@ -3,7 +3,6 @@ #include - 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) { diff --git a/tests/test.h b/tests/test.h index 9d20751..b021718 100644 --- a/tests/test.h +++ b/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*); diff --git a/tests/unit_test.c b/tests/unit_test.c index d800ab3..b59ccb3 100644 --- a/tests/unit_test.c +++ b/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 }, diff --git a/xim/xim.c b/xim/xim.c index 8953371..abeef61 100644 --- a/xim/xim.c +++ b/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;