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

Peter Hutterer peter.hutterer at who-t.net
Tue Aug 14 16:20:07 PDT 2012


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.

Cheers,
   Peter



More information about the xorg-devel mailing list