user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* [PATCH 14/18] xap_helper: stricter and harsher error handling
  2023-11-13 13:15  7% [PATCH 00/18] cindex: some --associate work Eric Wong
@ 2023-11-13 13:15  3% ` Eric Wong
  0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2023-11-13 13:15 UTC (permalink / raw)
  To: meta

We'll require an error stream for dump_ibx and dump_roots
commands; they're too important to ignore.  Instead of writing
code to provide diagnostics for errors, rely on abort(3) and the
-ggdb3 compiler flag to generate nice core dumps for gdb since
all commands sent to xap_helper are from internal users.
We'll even abort on most usage errors since they could be
bugs in split2argv or our use of getopt(3).

We'll also just exit on ENOMEM errors since it's the easiest way
to recover from those errors by starting a new process which
closes all open Xapian DB handles.
---
 lib/PublicInbox/XapHelper.pm |  28 ++---
 lib/PublicInbox/xap_helper.h | 202 ++++++++++++++---------------------
 t/xap_helper.t               |   5 +-
 3 files changed, 95 insertions(+), 140 deletions(-)

diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index 4157600f..428b732e 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -16,6 +16,7 @@ use PublicInbox::DS qw(awaitpid);
 use autodie qw(open getsockopt);
 use POSIX qw(:signal_h);
 use Fcntl qw(LOCK_UN LOCK_EX);
+use Carp qw(croak);
 my $X = \%PublicInbox::Search::X;
 our (%SRCH, %WORKERS, $nworker, $workerset, $in);
 our $stderr = \*STDERR;
@@ -70,14 +71,14 @@ sub dump_ibx_iter ($$$) {
 
 sub emit_mset_stats ($$) {
 	my ($req, $mset) = @_;
-	my $err = $req->{1} or return;
+	my $err = $req->{1} or croak "BUG: caller only passed 1 FD";
 	say $err 'mset.size='.$mset->size.' nr_out='.$req->{nr_out}
 }
 
 sub cmd_dump_ibx {
 	my ($req, $ibx_id, $qry_str) = @_;
-	$qry_str // return warn('usage: dump_ibx [OPTIONS] IBX_ID QRY_STR');
-	$req->{A} or return warn('dump_ibx requires -A PREFIX');
+	$qry_str // die 'usage: dump_ibx [OPTIONS] IBX_ID QRY_STR';
+	$req->{A} or die 'dump_ibx requires -A PREFIX';
 	my $max = $req->{'m'} // $req->{srch}->{xdb}->get_doccount;
 	my $opt = { relevance => -1, limit => $max, offset => $req->{o} // 0 };
 	$opt->{eidx_key} = $req->{O} if defined $req->{O};
@@ -88,9 +89,7 @@ sub cmd_dump_ibx {
 			$t = dump_ibx_iter($req, $ibx_id, $it) // $t;
 		}
 	}
-	if (my $err = $req->{1}) {
-		say $err 'mset.size='.$mset->size.' nr_out='.$req->{nr_out}
-	}
+	emit_mset_stats($req, $mset);
 }
 
 sub dump_roots_iter ($$$) {
@@ -120,9 +119,8 @@ sub dump_roots_flush ($$) {
 
 sub cmd_dump_roots {
 	my ($req, $root2id_file, $qry_str) = @_;
-	$qry_str // return
-		warn('usage: dump_roots [OPTIONS] ROOT2ID_FILE QRY_STR');
-	$req->{A} or return warn('dump_roots requires -A PREFIX');
+	$qry_str // die 'usage: dump_roots [OPTIONS] ROOT2ID_FILE QRY_STR';
+	$req->{A} or die 'dump_roots requires -A PREFIX';
 	open my $fh, '<', $root2id_file;
 	my $root2id; # record format: $OIDHEX "\0" uint32_t
 	my @x = split(/\0/, read_all $fh);
@@ -150,7 +148,7 @@ sub dispatch {
 	my ($req, $cmd, @argv) = @_;
 	my $fn = $req->can("cmd_$cmd") or return;
 	$GLP->getoptionsfromarray(\@argv, $req, @SPEC) or return;
-	my $dirs = delete $req->{d} or return warn 'no -d args';
+	my $dirs = delete $req->{d} or die 'no -d args';
 	my $key = join("\0", @$dirs);
 	$req->{srch} = $SRCH{$key} //= do {
 		my $new = { qp_flags => $PublicInbox::Search::QP_FLAGS };
@@ -168,8 +166,7 @@ sub dispatch {
 		$new->{qp} = $new->qparse_new;
 		$new;
 	};
-	eval { $fn->($req, @argv) };
-	warn "E: $@" if $@;
+	$fn->($req, @argv);
 }
 
 sub recv_loop {
@@ -192,13 +189,10 @@ sub recv_loop {
 		my $i = 0;
 		open($req->{$i++}, '+<&=', $_) for @fds;
 		local $stderr = $req->{1} // \*STDERR;
-		if (chop($rbuf) ne "\0") {
-			warn "not NUL-terminated";
-			next;
-		}
+		die "not NUL-terminated" if chop($rbuf) ne "\0";
 		my @argv = split(/\0/, $rbuf);
 		$req->{nr_out} = 0;
-		eval { $req->dispatch(@argv) } if @argv;
+		$req->dispatch(@argv) if @argv;
 	}
 }
 
diff --git a/lib/PublicInbox/xap_helper.h b/lib/PublicInbox/xap_helper.h
index 8602170f..1380ffd0 100644
--- a/lib/PublicInbox/xap_helper.h
+++ b/lib/PublicInbox/xap_helper.h
@@ -78,6 +78,10 @@
 	assert(ckvar______ == (expect) && "BUG" && __FILE__ && __LINE__); \
 } while (0)
 
+// coredump on most usage errors since our only users are internal
+#define ABORT(...) do { warnx(__VA_ARGS__); abort(); } while (0)
+#define EABORT(...) do { warn(__VA_ARGS__); abort(); } while (0)
+
 // sock_fd is modified in signal handler, yes, it's SOCK_SEQPACKET
 static volatile int sock_fd = STDIN_FILENO;
 static sigset_t fullset, workerset;
@@ -145,10 +149,8 @@ struct worker {
 #define SPLIT2ARGV(dst,buf,len) split2argv(dst,buf,len,MY_ARRAY_SIZE(dst))
 static size_t split2argv(char **dst, char *buf, size_t len, size_t limit)
 {
-	if (buf[0] == 0 || len == 0 || buf[len - 1] != 0) {
-		warnx("bogus argument given");
-		return 0;
-	}
+	if (buf[0] == 0 || len == 0 || buf[len - 1] != 0)
+		ABORT("bogus argument given");
 	size_t nr = 0;
 	char *c = buf;
 	for (size_t i = 1; i < len; i++) {
@@ -156,11 +158,11 @@ static size_t split2argv(char **dst, char *buf, size_t len, size_t limit)
 			dst[nr++] = c;
 			c = buf + i + 1;
 		}
-		if (nr == limit) {
-			warnx("too many args: %zu", nr);
-			return 0;
-		}
+		if (nr == limit)
+			ABORT("too many args: %zu == %zu", nr, limit);
 	}
+	if (nr == 0) ABORT("no argument given");
+	if ((long)nr < 0) ABORT("too many args: %zu", nr);
 	return (long)nr;
 }
 
@@ -242,6 +244,8 @@ static void emit_mset_stats(struct req *req, const Xapian::MSet *mset)
 	if (req->fp[1])
 		fprintf(req->fp[1], "mset.size=%llu nr_out=%zu\n",
 			(unsigned long long)mset->size(), req->nr_out);
+	else
+		ABORT("BUG: %s caller only passed 1 FD", req->argv[0]);
 }
 
 static bool starts_with(const std::string *s, const char *pfx, size_t pfx_len)
@@ -290,20 +294,14 @@ static enum exc_iter dump_ibx_iter(struct req *req, const char *ibx_id,
 
 static bool cmd_dump_ibx(struct req *req)
 {
-	if ((optind + 1) >= req->argc) {
-		warnx("usage: dump_ibx [OPTIONS] IBX_ID QRY_STR");
-		return false; // need ibx_id + qry_str
-	}
-	if (!req->pfxc) {
-		warnx("dump_ibx requires -A PREFIX");
-		return false;
-	}
+	if ((optind + 1) >= req->argc)
+		ABORT("usage: dump_ibx [OPTIONS] IBX_ID QRY_STR");
+	if (!req->pfxc)
+		ABORT("dump_ibx requires -A PREFIX");
 
 	const char *ibx_id = req->argv[optind];
-	if (my_setlinebuf(req->fp[0])) { // for sort(1) pipe
-		perror("setlinebuf(fp[0])");
-		return false;
-	}
+	if (my_setlinebuf(req->fp[0])) // for sort(1) pipe
+		EABORT("setlinebuf(fp[0])"); // WTF?
 	req->asc = true;
 	req->sort_col = -1;
 	Xapian::MSet mset = mail_mset(req, req->argv[optind + 1]);
@@ -344,24 +342,22 @@ static void fbuf_ensure(void *ptr)
 {
 	struct fbuf *fbuf = (struct fbuf *)ptr;
 	if (fbuf->fp && fclose(fbuf->fp))
-		perror("fclose(fbuf->fp)");
+		err(EXIT_FAILURE, "fclose(fbuf->fp)"); // ENOMEM?
 	fbuf->fp = NULL;
 	free(fbuf->ptr);
 }
 
-static bool fbuf_init(struct fbuf *fbuf)
+static void fbuf_init(struct fbuf *fbuf)
 {
 	assert(!fbuf->ptr);
 	fbuf->fp = open_memstream(&fbuf->ptr, &fbuf->len);
-	if (fbuf->fp) return true;
-	perror("open_memstream(fbuf)");
-	return false;
+	if (!fbuf->fp) err(EXIT_FAILURE, "open_memstream(fbuf)");
 }
 
 static void xclose(int fd)
 {
 	if (close(fd) < 0 && errno != EINTR)
-		err(EXIT_FAILURE, "BUG: close");
+		EABORT("BUG: close");
 }
 
 #define CLEANUP_DUMP_ROOTS __attribute__((__cleanup__(dump_roots_ensure)))
@@ -372,19 +368,17 @@ static void dump_roots_ensure(void *ptr)
 		xclose(drt->root2id_fd);
 	hdestroy(); // idempotent
 	if (drt->mm_ptr && munmap(drt->mm_ptr, drt->sb.st_size))
-		err(EXIT_FAILURE, "BUG: munmap");
+		EABORT("BUG: munmap(%p, %zu)", drt->mm_ptr, drt->sb.st_size);
 	free(drt->entries);
 	fbuf_ensure(&drt->wbuf);
 }
 
 static bool root2ids_str(struct fbuf *root_ids, Xapian::Document *doc)
 {
-	if (!fbuf_init(root_ids)) return false;
-
-	bool ok = true;
 	Xapian::TermIterator cur = doc->termlist_begin();
 	Xapian::TermIterator end = doc->termlist_end();
 	ENTRY e, *ep;
+	fbuf_init(root_ids);
 	for (cur.skip_to("G"); cur != end; cur++) {
 		std::string tn = *cur;
 		if (!starts_with(&tn, "G", 1))
@@ -393,21 +387,16 @@ static bool root2ids_str(struct fbuf *root_ids, Xapian::Document *doc)
 		u.in = tn.c_str() + 1;
 		e.key = u.out;
 		ep = hsearch(e, FIND);
-		if (!ep) {
-			warnx("hsearch miss `%s'", e.key);
-			return false;
-		}
+		if (!ep) ABORT("hsearch miss `%s'", e.key);
 		// ep->data is a NUL-terminated string matching /[0-9]+/
 		fputc(' ', root_ids->fp);
 		fputs((const char *)ep->data, root_ids->fp);
 	}
 	fputc('\n', root_ids->fp);
-	if (ferror(root_ids->fp) | fclose(root_ids->fp)) {
-		perror("ferror|fclose(root_ids)");
-		ok = false;
-	}
+	if (ferror(root_ids->fp) | fclose(root_ids->fp))
+		err(EXIT_FAILURE, "ferror|fclose(root_ids)"); // ENOMEM
 	root_ids->fp = NULL;
-	return ok;
+	return true;
 }
 
 // writes term values matching @pfx for a given @doc, ending the line
@@ -440,20 +429,17 @@ static bool dump_roots_flush(struct req *req, struct dump_roots_tmp *drt)
 	bool ok = true;
 
 	if (!drt->wbuf.fp) return true;
-	if (fd < 0) err(EXIT_FAILURE, "BUG: fileno");
-	if (fclose(drt->wbuf.fp)) {
-		warn("fclose(drt->wbuf.fp)"); // malloc failure?
-		return false;
-	}
+	if (fd < 0) EABORT("BUG: fileno");
+	if (ferror(drt->wbuf.fp) | fclose(drt->wbuf.fp)) // ENOMEM?
+		err(EXIT_FAILURE, "ferror|fclose(drt->wbuf.fp)");
 	drt->wbuf.fp = NULL;
 	if (!drt->wbuf.len) goto done_free;
 	while (flock(drt->root2id_fd, LOCK_EX)) {
 		if (errno == EINTR) continue;
-		perror("LOCK_EX");
-		return false;
+		err(EXIT_FAILURE, "LOCK_EX"); // ENOLCK?
 	}
 	p = drt->wbuf.ptr;
-	do {
+	do { // write to client FD
 		ssize_t n = write(fd, p, drt->wbuf.len);
 		if (n > 0) {
 			drt->wbuf.len -= n;
@@ -465,10 +451,9 @@ static bool dump_roots_flush(struct req *req, struct dump_roots_tmp *drt)
 	} while (drt->wbuf.len);
 	while (flock(drt->root2id_fd, LOCK_UN)) {
 		if (errno == EINTR) continue;
-		perror("LOCK_UN");
-		return false;
+		err(EXIT_FAILURE, "LOCK_UN"); // ENOLCK?
 	}
-done_free:
+done_free: // OK to skip on errors, dump_roots_ensure calls fbuf_ensure
 	free(drt->wbuf.ptr);
 	drt->wbuf.ptr = NULL;
 	return ok;
@@ -501,7 +486,7 @@ static char *hsearch_enter_key(char *s)
 	// hdestroy frees each key on some platforms,
 	// so give it something to free:
 	char *ret = strdup(s);
-	if (!ret) perror("strdup");
+	if (!ret) err(EXIT_FAILURE, "strdup");
 	return ret;
 // AFAIK there's no way to detect musl, assume non-glibc Linux is musl:
 #elif defined(__GLIBC__) || defined(__linux__) || \
@@ -518,61 +503,42 @@ static bool cmd_dump_roots(struct req *req)
 {
 	CLEANUP_DUMP_ROOTS struct dump_roots_tmp drt = {};
 	drt.root2id_fd = -1;
-	if ((optind + 1) >= req->argc) {
-		warnx("usage: dump_roots [OPTIONS] ROOT2ID_FILE QRY_STR");
-		return false; // need file + qry_str
-	}
-	if (!req->pfxc) {
-		warnx("dump_roots requires -A PREFIX");
-		return false;
-	}
+	if ((optind + 1) >= req->argc)
+		ABORT("usage: dump_roots [OPTIONS] ROOT2ID_FILE QRY_STR");
+	if (!req->pfxc)
+		ABORT("dump_roots requires -A PREFIX");
 	const char *root2id_file = req->argv[optind];
 	drt.root2id_fd = open(root2id_file, O_RDONLY);
-	if (drt.root2id_fd < 0) {
-		warn("open(%s)", root2id_file);
-		return false;
-	}
-	if (fstat(drt.root2id_fd, &drt.sb)) {
-		warn("fstat(%s)", root2id_file);
-		return false;
-	}
+	if (drt.root2id_fd < 0)
+		EABORT("open(%s)", root2id_file);
+	if (fstat(drt.root2id_fd, &drt.sb)) // ENOMEM?
+		err(EXIT_FAILURE, "fstat(%s)", root2id_file);
 	// each entry is at least 43 bytes ({OIDHEX}\0{INT}\0),
 	// so /32 overestimates the number of expected entries by
 	// ~%25 (as recommended by Linux hcreate(3) manpage)
 	size_t est = (drt.sb.st_size / 32) + 1; //+1 for "\0" termination
-	if ((uint64_t)drt.sb.st_size > (uint64_t)SIZE_MAX) {
-		warnx("%s size too big (%lld bytes > %zu)", root2id_file,
-			(long long)drt.sb.st_size, SIZE_MAX);
-		return false;
-	}
+	if ((uint64_t)drt.sb.st_size > (uint64_t)SIZE_MAX)
+		err(EXIT_FAILURE, "%s size too big (%lld bytes > %zu)",
+			root2id_file, (long long)drt.sb.st_size, SIZE_MAX);
 	drt.mm_ptr = mmap(NULL, drt.sb.st_size, PROT_READ,
 				MAP_PRIVATE, drt.root2id_fd, 0);
-	if (drt.mm_ptr == MAP_FAILED) {
-		warn("mmap(%s)", root2id_file);
-		return false;
-	}
+	if (drt.mm_ptr == MAP_FAILED)
+		err(EXIT_FAILURE, "mmap(%zu, %s)", drt.sb.st_size,root2id_file);
 	drt.entries = (char **)calloc(est * 2, sizeof(char *));
-	if (!drt.entries) {
-		warn("calloc(%zu * 2, %zu)", est, sizeof(char *));
-		return false;
-	}
+	if (!drt.entries)
+		err(EXIT_FAILURE, "calloc(%zu * 2, %zu)", est, sizeof(char *));
 	size_t tot = split2argv(drt.entries, (char *)drt.mm_ptr,
 				drt.sb.st_size, est * 2);
 	if (tot <= 0) return false; // split2argv already warned on error
-	if (!hcreate(est)) {
-		warn("hcreate(%zu)", est);
-		return false;
-	}
+	if (!hcreate(est))
+		err(EXIT_FAILURE, "hcreate(%zu)", est);
 	for (size_t i = 0; i < tot; ) {
 		ENTRY e;
-		e.key = hsearch_enter_key(drt.entries[i++]);
-		if (!e.key) return false;
+		e.key = hsearch_enter_key(drt.entries[i++]); // dies on ENOMEM
 		e.data = drt.entries[i++];
-		if (!hsearch(e, ENTER)) {
-			warn("hsearch(%s => %s, ENTER)", e.key,
-				(const char *)e.data);
-			return false;
-		}
+		if (!hsearch(e, ENTER))
+			err(EXIT_FAILURE, "hsearch(%s => %s, ENTER)", e.key,
+					(const char *)e.data);
 	}
 	req->asc = true;
 	req->sort_col = -1;
@@ -581,8 +547,8 @@ static bool cmd_dump_roots(struct req *req)
 	// @UNIQ_FOLD in CodeSearchIdx.pm can handle duplicate lines fine
 	// in case we need to retry on DB reopens
 	for (Xapian::MSetIterator i = mset.begin(); i != mset.end(); i++) {
-		if (!drt.wbuf.fp && !fbuf_init(&drt.wbuf))
-			return false;
+		if (!drt.wbuf.fp)
+			fbuf_init(&drt.wbuf);
 		for (int t = 10; t > 0; --t)
 			switch (dump_roots_iter(req, &drt, &i)) {
 			case ITER_OK: t = 0; break; // leave inner loop
@@ -672,6 +638,8 @@ again:
 
 	// success! no signals for the rest of the request/response cycle
 	CHECK(int, 0, sigprocmask(SIG_SETMASK, &fullset, NULL));
+	if (r > 0 && msg.msg_flags)
+		ABORT("unexpected msg_flags");
 
 	*len = r;
 	if (cmsg.hdr.cmsg_level == SOL_SOCKET &&
@@ -806,7 +774,7 @@ static void dispatch(struct req *req)
 			break;
 		}
 	}
-	if (!req->fn) goto cmd_err;
+	if (!req->fn) ABORT("not handled: `%s'", req->argv[0]);
 
 	kfp = open_memstream(&fbuf.ptr, &size);
 	// write padding, first
@@ -825,53 +793,48 @@ static void dispatch(struct req *req)
 		case 'd': fwrite(optarg, strlen(optarg) + 1, 1, kfp); break;
 		case 'k':
 			req->sort_col = strtol(optarg, &end, 10);
-			if (*end) goto cmd_err;
+			if (*end) ABORT("-k %s", optarg);
 			switch (req->sort_col) {
-			case LONG_MAX: case LONG_MIN: goto cmd_err;
+			case LONG_MAX: case LONG_MIN: ABORT("-k %s", optarg);
 			}
 			break;
 		case 'm':
 			req->max = strtoull(optarg, &end, 10);
-			if (*end) goto cmd_err;
-			if (req->max == ULLONG_MAX) goto cmd_err;
+			if (*end || req->max == ULLONG_MAX)
+				ABORT("-m %s", optarg);
 			break;
 		case 'o':
 			req->off = strtoull(optarg, &end, 10);
-			if (*end) goto cmd_err;
-			if (req->off == ULLONG_MAX) goto cmd_err;
+			if (*end || req->off == ULLONG_MAX)
+				ABORT("-o %s", optarg);
 			break;
 		case 'r': req->relevance = true; break;
 		case 't': req->collapse_threads = true; break;
 		case 'A':
 			req->pfxv[req->pfxc++] = optarg;
-			if (MY_ARG_MAX == req->pfxc) goto cmd_err;
+			if (MY_ARG_MAX == req->pfxc)
+				ABORT("too many -A");
 			break;
 		case 'O': req->Oeidx_key = optarg - 1; break; // pad "O" prefix
 		case 'T':
 			req->timeout_sec = strtoul(optarg, &end, 10);
-			if (*end) goto cmd_err;
-			if (req->timeout_sec == ULONG_MAX) goto cmd_err;
+			if (*end || req->timeout_sec == ULONG_MAX)
+				ABORT("-T %s", optarg);
 			break;
-		default: goto cmd_err;
+		default: ABORT("bad switch `-%c'", c);
 		}
 	}
-	if (ferror(kfp) | fclose(kfp)) {
-		perror("ferror|fclose");
-		goto cmd_err;
-	}
+	if (ferror(kfp) | fclose(kfp))
+		err(EXIT_FAILURE, "ferror|fclose"); // likely ENOMEM
 	fbuf.srch->db = NULL;
 	fbuf.srch->qp = NULL;
 	fbuf.srch->paths_len = size - offsetof(struct srch, paths);
 	if (fbuf.srch->paths_len <= 0) {
 		free_srch(fbuf.srch);
-		warnx("no -d args");
-		goto cmd_err;
+		ABORT("no -d args");
 	}
 	s = (struct srch **)tsearch(fbuf.srch, &srch_tree, srch_cmp);
-	if (!s) {
-		perror("tsearch");
-		goto cmd_err;
-	}
+	if (!s) err(EXIT_FAILURE, "tsearch"); // likely ENOMEM
 	req->srch = *s;
 	if (req->srch != fbuf.srch) { // reuse existing
 		free_srch(fbuf.srch);
@@ -881,11 +844,11 @@ static void dispatch(struct req *req)
 		void *del = tdelete(fbuf.srch, &srch_tree, srch_cmp);
 		assert(del);
 		free_srch(fbuf.srch);
-		goto cmd_err;
+		goto cmd_err; // srch_init already warned
 	}
 	try {
 		if (!req->fn(req))
-			goto cmd_err;
+			warnx("`%s' failed", req->argv[0]);
 	} catch (const Xapian::Error & e) {
 		warnx("Xapian::Error: %s", e.get_description().c_str());
 	} catch (...) {
@@ -924,7 +887,7 @@ static void stderr_restore(FILE *tmp_err)
 	return;
 #endif
 	if (ferror(stderr) | fflush(stderr))
-		perror("ferror|fflush stderr");
+		err(EXIT_FAILURE, "ferror|fflush stderr");
 	while (dup2(orig_err_fd, STDERR_FILENO) < 0) {
 		if (errno != EINTR)
 			err(EXIT_FAILURE, "dup2(%d => 2)", orig_err_fd);
@@ -953,8 +916,7 @@ static void recv_loop(void) // worker process loop
 		if (req.fp[1])
 			stderr_set(req.fp[1]);
 		req.argc = (int)SPLIT2ARGV(req.argv, rbuf, len);
-		if (req.argc > 0)
-			dispatch(&req);
+		dispatch(&req);
 		if (ferror(req.fp[0]) | fclose(req.fp[0]))
 			perror("ferror|fclose fp[0]");
 		if (req.fp[1]) {
@@ -1066,7 +1028,7 @@ static void reaped_worker(pid_t pid, int st)
 	if (WIFEXITED(st) && WEXITSTATUS(st) == EX_NOINPUT)
 		alive = false;
 	else if (st)
-		warnx("worker[%lu] died $?=%d", nr, st);
+		warnx("worker[%lu] died $?=%d alive=%d", nr, st, (int)alive);
 	if (alive)
 		start_workers();
 }
diff --git a/t/xap_helper.t b/t/xap_helper.t
index 83f59d7d..9e0b234d 100644
--- a/t/xap_helper.t
+++ b/t/xap_helper.t
@@ -47,11 +47,10 @@ is(scalar(@int), 1, 'have 1 internal shard') or diag explain(\@int);
 
 my $doreq = sub {
 	my ($s, @arg) = @_;
-	my $err = pop @arg if ref($arg[-1]);
+	my $err = ref($arg[-1]) ? pop(@arg) : \*STDERR;
 	pipe(my $x, my $y);
 	my $buf = join("\0", @arg, '');
-	my @fds = fileno($y);
-	push @fds, fileno($err) if $err;
+	my @fds = (fileno($y), fileno($err));
 	my $n = $PublicInbox::IPC::send_cmd->($s, \@fds, $buf, 0) //
 		xbail "send: $!";
 	my $exp = length($buf);

^ permalink raw reply related	[relevance 3%]

* [PATCH 00/18] cindex: some --associate work
@ 2023-11-13 13:15  7% Eric Wong
  2023-11-13 13:15  3% ` [PATCH 14/18] xap_helper: stricter and harsher error handling Eric Wong
  0 siblings, 1 reply; 2+ results
From: Eric Wong @ 2023-11-13 13:15 UTC (permalink / raw)
  To: meta

Still very much in flux, but some treewide cleanups in there...

And I've been wondering if "join" is a better word than
"associate" to denote the relationship between inboxes
and coderepos.

But "join" (even if we use join(1) internally) probably
implies strict relationships, whereas our current "associate"
is always going to be fuzzy due to patchids being fuzzy
and blobs OIDs being abbreviated in patches.

I'm also thinking about moving --associate-* CLI switches
into suboptions (e.g. what getsubopt(3) supports), so:

	--associate=aggressive,prefixes=patchid+dfblob

But Perl doesn't ship with getsubopt(3) emulation
out-of-the-box

Eric Wong (18):
  cindex: check `say' errors w/ close or ->flush
  tmpfile: check `stat' errors, use autodie for unlink
  cindex: use `local' for pipes between processes
  xap_helper_cxx: use write_file helper
  xap_helper_cxx: make the build process ccache-friendly
  xap_helper_cxx: use -pipe by default in CXXFLAGS
  xap_client: spawn C++ xap_helper directly
  treewide: update read_all to avoid eof|close checks
  spawn: don't append to scalarrefs on stdout/stderr
  cindex: imply --all with --associate w/o -I/--only
  cindex: delay associate until prune+indexing finish
  xap_helper: Perl dump_ibx respects `-m MAX'
  cidx_xap_helper_aux: complain about truncated inputs
  xap_helper: stricter and harsher error handling
  xap_helper: better variable naming for key buffer
  cindex: do not guess integer maximum for Xapian
  cindex: rename associate-max => window
  cindex: support --associate-aggressive shortcut

 lib/PublicInbox/CidxComm.pm         |   6 +-
 lib/PublicInbox/CidxXapHelperAux.pm |   6 +-
 lib/PublicInbox/CodeSearchIdx.pm    | 122 ++++++++++-----
 lib/PublicInbox/Gcf2.pm             |   3 +-
 lib/PublicInbox/IO.pm               |  18 ++-
 lib/PublicInbox/LeiInput.pm         |  10 +-
 lib/PublicInbox/LeiMirror.pm        |  10 +-
 lib/PublicInbox/LeiToMail.pm        |   3 +-
 lib/PublicInbox/Spawn.pm            |   4 +-
 lib/PublicInbox/TestCommon.pm       |   6 +-
 lib/PublicInbox/Tmpfile.pm          |  10 +-
 lib/PublicInbox/XapClient.pm        |  28 ++--
 lib/PublicInbox/XapHelper.pm        |  30 ++--
 lib/PublicInbox/XapHelperCxx.pm     |  55 +++----
 lib/PublicInbox/xap_helper.h        | 233 ++++++++++++----------------
 script/public-inbox-cindex          |   3 +-
 script/public-inbox-learn           |   2 +-
 script/public-inbox-mda             |   2 +-
 script/public-inbox-purge           |   2 +-
 t/spawn.t                           |   2 +-
 t/xap_helper.t                      |  27 ++--
 21 files changed, 287 insertions(+), 295 deletions(-)

Yay, less code!

^ permalink raw reply	[relevance 7%]

Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2023-11-13 13:15  7% [PATCH 00/18] cindex: some --associate work Eric Wong
2023-11-13 13:15  3% ` [PATCH 14/18] xap_helper: stricter and harsher error handling Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).