On 2023/11/03 16:00:32 +0100, Theo Buehler <tb@theobuehler.org> wrote:
> On Fri, Nov 03, 2023 at 03:48:45PM +0100, Omar Polo wrote:
> > On 2023/11/03 11:01:50 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> > > On Thu, Oct 26, 2023 at 09:20:13AM +0200, Omar Polo wrote:
> > > > Should we also start to use ibuf_fd_set() instead of
> > > > reaching into the struct?
> > > 
> > > Sure, why not.
> > 
> > Then the diff would be this.  I'm not particularly compelled to commit
> > it, since the main interesting thing about ibuf_fd_set() seems to be
> > that it closes the the previous fd passed (if any).  Here we always
> > create a fresh ibuf and send it.
> 
> I think it would be best if got worked under the assumption that ibufs
> are opaque, even if this may not actually happen in libutil.

ah ok, good to know, I'll go ahead with this then.

> I'm also not sure why got always does wbuf->fd = -1 before imsg_close().
> This fd is already -1 from imsg_create() (via ibuf_dynamic()).

don't know either, but I've dropped them all just before sending this
diff.

https://got.omarpolo.com/?action=diff&commit=ac4f092c7c8e090ee733c90b8b0f0274872c8662&headref=HEAD&path=got.git

> > 
> > diff /home/op/w/got
> > commit - ac4f092c7c8e090ee733c90b8b0f0274872c8662
> > path + /home/op/w/got
> > blob - 7d479df74800ae6ac1809d68db5f4d54482efd23
> > file + lib/privsep.c
> > --- lib/privsep.c
> > +++ lib/privsep.c
> > @@ -419,7 +419,7 @@ got_privsep_send_tree_req(struct imsgbuf *ibuf, int fd
> >  		}
> >  	}
> >  
> > -	wbuf->fd = fd;
> > +	ibuf_fd_set(wbuf, fd);
> >  	imsg_close(ibuf, wbuf);
> >  
> >  	return flush_imsg(ibuf);
> > @@ -612,7 +612,7 @@ got_privsep_send_fetch_req(struct imsgbuf *ibuf, int f
> >  			return err;
> >  		}
> >  	}
> > -	wbuf->fd = fd;
> > +	ibuf_fd_set(wbuf, fd);
> >  	fd = -1;
> >  	imsg_close(ibuf, wbuf);
> >  
> >