[PATCH xorg-gtest] process: document that va_args for Start() must end with zero-length string

[Chase Douglas]

On 08/14/2012 04:20 PM, Peter Hutterer wrote:
> On Tue, Aug 14, 2012 at 10:19:00AM -0700, Chase Douglas wrote:
>> On 08/13/2012 06:16 PM, Peter Hutterer wrote:
>>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>>> ---
>>>   include/xorg/gtest/xorg-gtest-process.h | 6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/xorg/gtest/xorg-gtest-process.h b/include/xorg/gtest/xorg-gtest-process.h
>>> index 69b3b34..8cf60e3 100644
>>> --- a/include/xorg/gtest/xorg-gtest-process.h
>>> +++ b/include/xorg/gtest/xorg-gtest-process.h
>>> @@ -123,7 +123,8 @@ class Process {
>>>      * See 'man execvp' for further information on the variadic argument list.
>>>      *
>>>      * @param program The program to start.
>>> -   * @param args Variadic list of arguments passed to the program.
>>> +   * @param args Variadic list of arguments passed to the program. This list
>>> +   * must end in a zero-length string ("", not NULL).
>>>      *
>>>      * @throws std::runtime_error on failure.
>>>      *
>>> @@ -135,7 +136,8 @@ class Process {
>>>     /**
>>>      * Starts a program as a child process.
>>>      *
>>> -   * Takes a variadic list of arguments passed to the program.
>>> +   * Takes a variadic list of arguments passed to the program. This list
>>> +   * must end in a zero-length string ("", not NULL).
>>>      * See 'man execvp' for further information on the variadic argument list.
>>>      *
>>>      * @param program The program to start.
>>>
>>
>> Hmmm... I wish we had though to use a NULL sentinel for the varargs
>> version. We could have used the gcc sentinel function attribute to
>> help warn people of bad usage. I suppose it's too late to change now
>> that we've released it.
>
> I disagree. We're only up to 0.4 and have few users only. Pretending we have
> a good and stable API already is optimistic at best, it's better to fix the
> things that are obviously (or at least reasonably :) broken. This isn't hard
> thing to fix either (I'll send patches out in a bit), so there really isn't
> an excuse for having a bad API, it'll just come and haunt us later.

I'll confess that I was attempting a bait a little with my response :). 
I agree that there are few users and the change would be small, so I'm 
fine with fixing this up.

-- Chase