Re: [PATCH 1/6] isolation, util: Fold close_open_files() into isolate_initial()

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2204 bytes --]

On Wed, Jul 15, 2026 at 09:12:23AM +0200, Stefano Brivio wrote:
> On Tue, 14 Jul 2026 19:29:21 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > Most functions in util.c are, well, utilities, that are useful in a bunch
> > of places.  close_open_files(), however, is very specific, it's only called
> > from isolate_initial(), and really only can be called from there - it's
> 
> Nit: its

Fixed.

> > logic encodes a very specific action we want to take at that point.
> > 
> > So, remove it from util.c and fold into isolate_initial().
> 
> To me this doesn't actually look like an improvement (well, of course,
> otherwise I would have structured commit 09603cab28f9 ("passt, util:
> Close any open file that the parent might have leaked") differently)
> exactly because close_open_files() is very specific and does exactly
> that one thing, contributing to a shorter version of isolate_initial().
> 
> Now, I realise that we're pretty far from "fixing"
> https://bugs.passt.top/show_bug.cgi?id=31, but still,
> isolate_initial() in its current form looks much more digestible than
> a version with thirty-three lines added on top. It currently has a
> specific role and that's conceptually a separated operation,
> regardless of the fact it has a single caller.

> On top of that, the function comment to close_open_files() disappears
> this way, because it has no natural place anymore. But I think that
> was rather useful.

Fair points.  I still think the case for putting this code in
isolate.c rather than util.c is strong.  How would you feel about
moving close_open_files() to isolate.c, but not folding it into
isolate_initial().  Maybe 'isolate_fds()'?

> 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> By the way, should you respin, you could probably Cc: Ammar as he
> volunteered to review this kind of series. I would also suggest Cc'ing
> reporters (and marking them as such when doable).

Oh, good point.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]