[llvm-commits] [llvm] r131186 - in /llvm/trunk: include/llvm/Support/Program.h lib/Support/Program.cpp lib/Support/Unix/Program.inc tools/bugpoint/ExecutionDriver.cpp tools/bugpoint/Miscompilation.cpp tools/bugpoint/ToolRunner.cpp

[Andrew Trick]

On May 19, 2011, at 10:50 AM, Dan Gohman wrote:

> 
> On May 18, 2011, at 5:03 PM, Andrew Trick wrote:
> 
>> On May 18, 2011, at 2:42 PM, Dan Gohman wrote:
>> 
>>>> --- llvm/trunk/include/llvm/Support/Program.h (original)
>>>> +++ llvm/trunk/include/llvm/Support/Program.h Wed May 11 11:31:24 2011
>>>> @@ -96,9 +96,11 @@
>>>>    ///< expires, the child is killed and this call returns. If zero,
>>>>    ///< this function will wait until the child finishes or forever if
>>>>    ///< it doesn't.
>>>> -      std::string* ErrMsg ///< If non-zero, provides a pointer to a string
>>>> +      std::string* ErrMsg, ///< If non-zero, provides a pointer to a string
>>>>    ///< instance in which error messages will be returned. If the string
>>>>    ///< is non-empty upon return an error occurred while waiting.
>>>> +      const char *SignalPrefix ///< If non-zero, provides a prefix to be
>>>> +      ///< prepended to ErrMsg if the process is terminated abnormally.
>>>>    );
>>> 
>>> This function is documented to return a negative value if the process is
>>> terminated abnormally. Shouldn't the task of producing a pretty error
>>> message belong to client code, rather than being built into these low-level
>>> support functions?
>> 
>> 
>> Ah. I knew I wouldn't get away with this for long. The docs don't cleanly specify the behavior. I needed a way in the client, many levels above to distinguish between signals and other sorts of abnormal events like timeouts and errors spawning the process. I was extremely paranoid about changing the return value and breaking other clients, which I don't know how to test and are likely outside the main source. So I went with an approach that I can prove is transparent to other clients. I'd be thrilled to drop SignalPrefix if anyone with more experience can tell me how to do it safely.
> 
> 
> Actually, it looks like Wait doesn't actually implement what its comment
> says. It never returns the signal number; it just returns -1 in the
> WIFSIGNALED case. So one solution would be to just change the documentation
> to match this, and then use -2 for your purposes.

Yes, that's what I would have liked to do and exactly what I meant by "paranoid about changing the return value". Another thing I was paranoid about is the platform independence of the API.

Looking at the in-tree users, it seems ok. If you think it's ok in general, I'll go ahead with it.

My main problem here is that if I do break someone's process management, I'm not likely to find out about it.

-Andy