From: Omar Polo Subject: Re: convert imsg->fd to imsg_get_fd() To: Stefan Sperling Cc: gameoftrees@openbsd.org Date: Wed, 17 Jan 2024 19:33:00 +0100 On 2024/01/17 17:52:14 +0100, Stefan Sperling wrote: > On Wed, Jan 17, 2024 at 02:13:20PM +0100, Omar Polo wrote: > > On 2024/01/17 13:10:39 +0100, Stefan Sperling wrote: > > > On Wed, Jan 17, 2024 at 12:58:01PM +0100, Stefan Sperling wrote: > > > > On Wed, Jan 17, 2024 at 12:01:06PM +0100, Omar Polo wrote: > > > > > Oh, this also means that got can't be built on the latest release. > > > > > > > > The main branch already fails to build on 7.4 now. > > > > > > > > I would like to preserve our ability to provide -stable packages on 7.4. > > > > Is the hack below too horrible or can we use this until 7.5? > > > > > > New diff which also fixes the build of gotd, gotsh, and gotwebd > > > on 7.4 by adding imsg_get_fd() stubs in additional places. > > > > I concur with the sentiment but the implementation is wrong. We leak > > fds if imsg_free() doesn't close them, which is the case in 7.4. > > > > Here's another way to do this, with less fiddling. Still ugly, but > > should avoid the leaks. We'll need to augment this with > > imsg_get_data() and _len() soon. > > This reaching into libutil internals feels a bit too risky to me. > And it is probably not worth the extra effort. > Right now there is nothing critical to fix relative to 0.95. > These fd leaks in privsep helpers have always been there and they > don't seem to really hurt anyone. that leak is not a big deal since we don't reuse the got-read-pack processes. > How about we shelve this effort, for now. If something comes up that > absolutely needs to be patched in -stable, we can always apply a patch > against the 0.95 package in -stable while moving -current to 0.96. Agreed. If we really must update -stable we could also consider just bundling imsg{,-buffer}.c and imsg.h. It's also quite difficult to "port" imsg_get_data(), imsg_get_len(), etc. as shims on top of previous versions of libutil. I'll just give up and update my server to -current one of these days :) > We should just make sure to move our regression buildbots to -current.