Re: [gentoo-portage-dev] [5/7] portage_exec cleanups

[Jason Stubbs]

On Sunday 30 October 2005 02:17, Brian Harring wrote:
> On Sun, Oct 30, 2005 at 01:40:41AM +0900, Jason Stubbs wrote:
> > Even with the fd_pipes, the try/except block in there covers a bug that
> > is hit every time it is entered.
>
> *cough* really doesn't surprise me. :)
>
> > > Looking through the 2.4 subprocess module (which came along after the
> > > mods in question), adding an option controlling the fd cleansing would
> > > make sense, as long as the default option is to close the fds.
> >
> > Given the threading comment, I'd be agreeable to this. However, the spawn
> > of tee would have to specifically close the write side of the pipe when
> > unknown FDs aren't closed. Is having two extra paramaters, close_fds (a
> > list) and close_all_fds (a bool), to spawn okay?
>
> It's needed for any logged calls, doesn't mean I like it though.  How
> about this,
> def spawn(...close_fds=True...):
> 	if close_fds is True:
> 		# kill all that aren't marked for open
> 	elif close_fds:
> 		# must be an iterable of what to kill
> 	else:
> 		#whatever code required to keep it from nuking file handles
>
> Don't really like that one much myself, but it would allow for the
> logging to use the existing structure, and one less option on the
> overgrown spawn* prototype(s).

Yep, this sort of messyness is why I chose to back down. ;)

> sidenote, since max_fd_limit needs to survive, tag
>  try:
> 	import resource
> -	max_fd_limit=resource.getrlimit(RLIMIT_NOFILE)
> +	max_fd_limit=resource.getrlimit(resource.RLIMIT_NOFILE)
>  except SystemExit, e:
> 	raise
>
> in while you're at it, since that's obviously broke.  Current code
> would default to range(256), but default is 1024 fds for gentoo boxes
> (so that _is_ a hole).

Got that one already, but I didn't want to point out too many bugs. ;)
(There's still one more you missed there, btw; getrlimit() returns a tuple)

--
Jason Stubbs
-- 
gentoo-portage-dev@g.o mailing list