On Thu, Dec 29, 2022 at 12:55:03PM +0100, Omar Polo wrote:
> > +	switch (sig) {
> > +	case SIGHUP:
> > +		break;
> > +	case SIGUSR1:
> > +		break;
> 
> just for curiosity, what's the point of catching SIGUSR1 too?  the
> listener process and gotwebd do that too (but gotwebd due to proc.c I
> guess), but otherwise they don't do anything with it.

No idea. This was just copied along from other daemons.
The HUP and USR signals are often used to trigger actions such as
reloading config files. I suspect that is why they are present in
all these signal handlers.

> > +	if (client->auth->iev.ibuf.fd != fd)
> > +		fatalx("%s: unexpected fd %d", __func__, fd);
> 
> it's a bit redundant to check the fd, since find_client_by_proc_fd
> should return a client with that fd.  what i'm missing?

Indeed. This is just a guard against accidents. I am not very happy
with find_client_by_proc_fd() because it is looping over the entire
client hash table. But I don't want to store a pointer inside struct
gotd_imsgev either, because this makes use-after-free bugs more likely
to happen. I am not sure yet if there is a better way.

Once we have all separate processes in place, we can do another sweep
of all the sanity checks and see which ones still make sense. There is
also the verify_imsg_src() function which is currently just being copied
along, and could eventually be dropped or refined.

> > +	if ((n = imsg_get(ibuf, &imsg)) == -1)
> > +		fatal("%s: imsg_get error", __func__);
> > +	if (n == 0)	/* No more messages. */
> > +		return;
> 
> any reason not to do the usual loop here around imsg_get?  it works
> like this, but if we ever grow the set of messages it could lead to
> issues, and looping would reduce the diff with the other dispatch
> functions.
> 
> (well, not that the auth process should do more, so it's fine like it
> is already)

The reason is that we expect the auth process to send us only one message
before we will dispose of it. I don't expect this to change, provided we
will not extend our authentication feature-set in the future.