Re: [gpsd-dev] [PATCH 4/4] Add Valgrind tests for gps library interface variants.

[Robert Norris]

Thanks for reviewing this patch.

I was less confident about it - just getting something to mostly work.

Since it doesn't get run automatically I didn't test with various scons config 
options such as qt=no, etc...

Yeh that LibError bit is indeed quite wrong!

Finally it is indeed QLibrary (not QtLibrary) - see for example: 
http://doc.qt.io/qt-4.8/qlibrary.html

I'll come up with an improved version.

--
Be Seeing You - Rob.
If at first you don't succeed,
then skydiving isn't for you.

________________________________________
From: gpsd-dev <address@hidden> on behalf of Fred Wright <address@hidden>
Sent: 12 February 2017 23:37:56
To: address@hidden
Subject: Re: [gpsd-dev] [PATCH 4/4] Add Valgrind tests for gps library 
interface variants.

I merged the first 3 patches in this set, but not this one, which has
multiple problems:

On Fri, 10 Feb 2017, Robert Norris wrote:
[...]
>  valgrind_audit = Utility('valgrind-audit',
> -    ['$SRCDIR/valgrind-audit.py', python_built_extensions, gpsd],
> +    ['$SRCDIR/valgrind-audit.py', python_built_extensions, gpsd, 
> test_libgps, test_gpsmm, test_qgpsmm],
>      '$PYTHON $SRCDIR/valgrind-audit.py'
>      )

This fails with libgpsmm=no and/or qt=no.  It needs to be careful not to
add things that are disabled.

[...]
> +    def spawn(self, port, test_program="", options="", background=False, 
> prefix=""):
[...]
> +        if not self.spawncmd:
> +            raise LibError("Cannot execute %s" % self.spawncmd)

If this ever hits, it will bomb attempting to substitute None for the %s.
It's actually pretty meaningless to include something known to be empty,
so it would make more sense to report test_program, along with a fallback
just in case it's unspecified.  e.g.:

        raise LibError("Cannot execute %s" % (test_program or "(null)"))

[...]
> +# QLibrary::setFileName
> +# QLibrary::setFileNameVersion
[...]

Should "QLibrary" be "QtLibrary"?

Fred Wright