Re: [libvirt] [PATCH] replace last instances of close()

>> Did I convince you? > Yes& no. On the surface it looks ok, but I'm still a little worried > about > the implications, because I put this code in there to avoid some nasty > re-entrancy problems. I guessing some later re-factoring might have fixed > those problems making this code redundant. So I'll ACK for nwo, but if we > notice any unusual behaviour with the monitor, then this is the place to > look... > The re-entrancy that was previously avoided with the 'closed' flag is now checked with the file descriptor being >= 0, both with the lock being held, so no concurrency is possible. The most dangerous thing IMO is that someone changes the locking later on for whatever reason. There really should be a comment on the functions reading and writing from/to the mon->fd that they need to be called with that lock held. Also that the 'mon-fd' is protected by the existing lock may be worth a comment. I'd push unless you want me to add comments to the file descriptor and functions requiring the lock being held.