[llvm-commits] [llvm] r81826 - in /llvm/trunk: include/llvm/System/Program.h lib/System/Unix/Program.inc lib/System/Win32/Program.inc

[Mikhail Glushenkov]

Hi Daniel,

On Mon, Sep 21, 2009 at 09:43:20PM -0700, Daniel Dunbar wrote:
> Hi Mikhail,
>
> This isn't safe, for exactly the same reasons as I fixed it before here:
>   http://llvm.org/viewvc/llvm-project?view=rev&revision=77953
> and it breaks ExecuteAndWait in exactly the same way. I'm reverting it
> for now. See http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090727/083264.html

Sorry, I haven't seen your previous e-mail. I don't quite understand what causes
the race condition, though...

> If the motivation is supporting GetProcessId pre XP then this should
> be recorded in the Program object.

"this" == PID? Yes, I guess that's better than any of the tricks from [1].

> I also see no reason to make a Program object copyable -- what would this
> mean? The program object should essentially be opaque, it is implemented by
> the System layer, which may well be non-copyable.

Well, if the Program object is just a PID, why not make it copyable? Clients may
want to store it somewhere (e.g. STL containers). Alternatively, we may add a
way to construct a Program from a PID (since there is already a GetPid()
method).

> I know we don't currently have a good way of running tests on Windows,
> but please verify that ExecuteAndWait doesn't break when modifying
> this code...

Sorry for that.

[1] http://www.codeproject.com/KB/threads/GettingProcessID.aspx