From: Claudio Jeker Subject: Re: rpki-client: free the msgbufs in main To: Theo Buehler Cc: tech@openbsd.org Date: Wed, 31 Dec 2025 13:24:38 +0100 On Wed, Dec 31, 2025 at 01:05:37PM +0100, Theo Buehler wrote: > On Wed, Dec 31, 2025 at 12:59:01PM +0100, Claudio Jeker wrote: > > On Wed, Dec 31, 2025 at 10:43:17AM +0100, Theo Buehler wrote: > > > These are four sizeable allocations (64k each) that running in filemode > > > with MALLOC_OPTIONS=-D shows. Not really an issue but since we also > > > close the corresponding file descriptors, we should probably also > > > release these. > > > > > > The remaining things flagged by -D in filemode are the global OIDs > > > from x509_init_oid() and the tals[] from tal_load_default(). > > > > > > Index: main.c > > > =================================================================== > > > RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v > > > diff -u -p -r1.302 main.c > > > --- main.c 13 Nov 2025 15:18:53 -0000 1.302 > > > +++ main.c 31 Dec 2025 09:37:16 -0000 > > > @@ -1485,9 +1485,15 @@ main(int argc, char *argv[]) > > > */ > > > > > > close(procfd); > > > + msgbuf_free(procq); > > > close(rsyncfd); > > > + msgbuf_free(rsyncq); > > > close(httpfd); > > > + msgbuf_free(httpq); > > > close(rrdpfd); > > > + msgbuf_free(rrdpq); > > > + procq = rsyncq = httpq = rrdpq = NULL; > > > + memset(queues, 0, sizeof(queues)); > > > > > > rc = 0; > > > for (;;) { > > > > > > > To be honest I think the better way is to check the noop flag and only > > fumble with most of those fds and msgbufs if those things are actually > > running. Right now in -n and -f mode most of those close call are with a > > -1 argument. > > > > Doing this correctly needs a bigger refactor since e.g. the NPFD usage > > needs to become dynamic. > > I don't disagree but I think this is independent of my change. Sure. I'm OK with landing your diff. I generally think that cleanup on exit is a very bad coding pattern. The kernel is normally better at freeing resources on exit. The close calls are done to signal the childs that the parent is gone which triggers them to exit. It is fine to also free the other bits of the communications channel since we really don't want to use them anymore. -- :wq Claudio