On Tue, Jan 10, 2023 at 12:22:02PM +0100, Omar Polo wrote:
> gotd_shutdown() calls exit so the code below now is dead.  Not a huge
> issue (just some close and a free) but worth noting.

Indeed. I have moved the free() up and removed closing of pack_fds which
are not even opened in the parent process anymore.

> > +	if (client->nref_updates != -1)
> > +		return got_error_msg(GOT_ERR_PRIVSEP_MSG, "begin_ref_updates 1");
> 
> fold; > 80

Oops, it was written this way aid debugging an issue. I have changed
this to just return got_error(GOT_ERR_PRIVSEP_MSG);

> > +	datalen = imsg->hdr.len - IMSG_HEADER_SIZE;
> > +	if (datalen != sizeof(istart))
> > +		return got_error(GOT_ERR_PRIVSEP_LEN);
> > +	memcpy(&istart, imsg->data, sizeof(istart));
> > +
> > +	if (istart.nref_updates <= 0)
> > +		return got_error_msg(GOT_ERR_PRIVSEP_MSG, "begin_ref_updates 2");
> 
> fold; > 80

Same.

> > +	memset(&imsg, 0, sizeof(imsg));
> 
> why zeroing imsg?

I guess ever since OpenBSD 7.1 errata 001 I just do this automatically,
everywhere...