[llvm] r209951 - Fix the behavior of ExecuteAndWait with a non-zero timeout.

[Alp Toker]

The real problem here is that the sys::Wait() prototype has unclear 
semantics but the commit only fixes the symptom.

SecondsToWait and WaitUntilTerminates are mutually exclusive, so having 
SecondsToWait=-1 signify WaitUntilTerminates and removing the 
WaitUntilTerminates parameter is a better way to make the interface robust.

With that change it'll be easier to inspect the code for correctness, 
and you may consider removing/disabling the unit test which steals a 
second of testing time that could be better used to run a few thousand 
other tests.

Alp.


On 31/05/2014 04:36, Peter Collingbourne wrote:
> Author: pcc
> Date: Fri May 30 20:36:02 2014
> New Revision: 209951
>
> URL: http://llvm.org/viewvc/llvm-project?rev=209951&view=rev
> Log:
> Fix the behavior of ExecuteAndWait with a non-zero timeout.
>
> Modified:
>      llvm/trunk/lib/Support/Program.cpp
>      llvm/trunk/unittests/Support/ProgramTest.cpp
>
> Modified: llvm/trunk/lib/Support/Program.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Program.cpp?rev=209951&r1=209950&r2=209951&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/Program.cpp (original)
> +++ llvm/trunk/lib/Support/Program.cpp Fri May 30 20:36:02 2014
> @@ -34,7 +34,8 @@ int sys::ExecuteAndWait(StringRef Progra
>     if (Execute(PI, Program, args, envp, redirects, memoryLimit, ErrMsg)) {
>       if (ExecutionFailed)
>         *ExecutionFailed = false;
> -    ProcessInfo Result = Wait(PI, secondsToWait, true, ErrMsg);
> +    ProcessInfo Result = Wait(
> +        PI, secondsToWait, /*WaitUntilTerminates=*/secondsToWait == 0, ErrMsg);
>       return Result.ReturnCode;
>     }
>   
>
> Modified: llvm/trunk/unittests/Support/ProgramTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/ProgramTest.cpp?rev=209951&r1=209950&r2=209951&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/Support/ProgramTest.cpp (original)
> +++ llvm/trunk/unittests/Support/ProgramTest.cpp Fri May 30 20:36:02 2014
> @@ -162,6 +162,36 @@ TEST(ProgramTest, TestExecuteNoWait) {
>     ASSERT_GT(LoopCount, 1u) << "LoopCount should be >1";
>   }
>   
> +TEST(ProgramTest, TestExecuteAndWaitTimeout) {
> +  using namespace llvm::sys;
> +
> +  if (getenv("LLVM_PROGRAM_TEST_TIMEOUT")) {
> +    sleep_for(/*seconds*/ 10);
> +    exit(0);
> +  }
> +
> +  std::string Executable =
> +      sys::fs::getMainExecutable(TestMainArgv0, &ProgramTestStringArg1);
> +  const char *argv[] = {
> +    Executable.c_str(),
> +    "--gtest_filter=ProgramTest.TestExecuteAndWaitTimeout",
> +    0
> +  };
> +
> +  // Add LLVM_PROGRAM_TEST_TIMEOUT to the environment of the child.
> +  std::vector<const char *> envp;
> +  CopyEnvironment(envp);
> +  envp.push_back("LLVM_PROGRAM_TEST_TIMEOUT=1");
> +  envp.push_back(0);
> +
> +  std::string Error;
> +  bool ExecutionFailed;
> +  int RetCode =
> +      ExecuteAndWait(Executable, argv, &envp[0], 0, /*secondsToWait=*/1, 0,
> +                     &Error, &ExecutionFailed);
> +  ASSERT_EQ(-2, RetCode);
> +}
> +
>   TEST(ProgramTest, TestExecuteNegative) {
>     std::string Executable = "i_dont_exist";
>     const char *argv[] = { Executable.c_str(), 0 };
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-- 
http://www.nuanti.com
the browser experts