Hello and Happy New Year! I wish all the best.

I apologize for the delay, I couldn't respond for the past 2 weeks.

On 14:39 Wed 29 Dec     , Michael Hanselmann wrote:
> Hi Apollon
> 
> Am 21. Dezember 2010 14:53 schrieb Apollon Oikonomopoulos
> <[email protected]>:
> > That was the test case I was thinking of too (see cover letter).
> >
> > Revised patch with unittest follows.
> 
> Thanks for the update and sorry for the delay.
> 
> > +  def testNocloseFds(self):
> > +    """Test selective fd retention (noclose_fds)"""
> > +    temp = open(self.fname, "w")
> > +    temp.write("test")
> > +    temp.flush()
> 
> You should seek to the file's beginning here, no?

No, it's not necessary. open(2)'ing /dev/fd/<nr> obtains a new file 
descriptor with a separate file pointer to the same inode as fd <nr>.  
Technically it is not the same as reading directly from fd <nr> (which 
would require resetting the read offset), but it is enough to test the 
desired functionality.

> > +    cmd = "cat /dev/fd/%d" % temp.fileno()
> 
> Could you please use the built-in “read”? "read -u %s </dev/null"

I'll resubmit for that.

> 
> > +    result = RunCmd(["/bin/bash", "-c", cmd])
> > +    self.assertEqual(result.exit_code, 1)
> 
> And seek again here.
See above.

> 
> > +    result = RunCmd(["/bin/bash", "-c", cmd], noclose_fds=[temp.fileno()])
> > +    self.assertEqual(result.exit_code, 0)
> > +    self.assertEqual(result.stdout.strip(), "test")
> 
> > +    temp.close()
> 
> Use:
> 
> temp = open(…)
> try:
>   temp.write(…)
>   …
> finally:
>   temp.close()
> 
Ack.

> Rest looks good.
> 
> Michael


Thanks,

Apollon

Reply via email to