[pdal-commits] [PDAL/PDAL] b9dfa7: Check for null pointers when using pg_query_once

GitHub noreply at github.com
Tue May 20 06:38:13 PDT 2014


  Branch: refs/heads/master
  Home:   https://github.com/PDAL/PDAL
  Commit: b9dfa798181720ebf886ee553911ae1be5f714df
      https://github.com/PDAL/PDAL/commit/b9dfa798181720ebf886ee553911ae1be5f714df
  Author: Pete Gadomski <pete.gadomski at gmail.com>
  Date:   2014-05-19 (Mon, 19 May 2014)

  Changed paths:
    M CMakeLists.txt
    M src/drivers/pgpointcloud/Reader.cpp
    M src/drivers/pgpointcloud/Writer.cpp
    M test/unit/CMakeLists.txt
    A test/unit/drivers/pgpointcloud/PgpointcloudWriterTest.cpp
    A test/unit/drivers/pgpointcloud/Support.hpp.in

  Log Message:
  -----------
  Check for null pointers when using pg_query_once

`pdal::drivers::pgpointcloud::pg_query_once` is C-ish -- it returns a
raw `char *` from `strdup`. In our driver code, we were not checking
that pointer against `NULL`, which lead to PDAL crashing when the target
postgres database existed, but did not have the [`pointcloud`
extension](https://github.com/pramsey/pointcloud) installed.

This patch updates all uses of `pg_query_once` in the codebase with a
check for pointer validity, throwing a `pdal_error` if the returned
pointer is `NULL`. Note that other functions in
`include/pdal/drivers/pgpointcloud/common.hpp` also return raw pointers,
but their usage has not been checked or modified for this patch, due
primarily to developer laziness but rationalized as commendable commit
cleanliness/avoidance of mission creep.

This patch also adds a test harness for the pgpointcloud drivers, which
helped verify the correctness of the fix. Since the unit tests require a
running postgres instance with a already-created database, the postgres
unit tests are disabled by default. They can be enabled by enabling the
dependent cmake option `WITH_PGPOINTCLOUD_TESTS`. The database
connection parameters can then be configured with CMake variables.

I think it would be better to allow configuration of the postgres test
database with some sort of configuration file (so a change to the
postgres test database configuration didn't require a rebuild), but
leveraging the existing CMake configuration system was quicker than
doing my own config parsing.

Fixes #292.


  Commit: 93283dd667d1d30fc510a15fd906626fcad15da7
      https://github.com/PDAL/PDAL/commit/93283dd667d1d30fc510a15fd906626fcad15da7
  Author: Howard Butler <howard at hobu.co>
  Date:   2014-05-20 (Tue, 20 May 2014)

  Changed paths:
    M CMakeLists.txt
    M src/drivers/pgpointcloud/Reader.cpp
    M src/drivers/pgpointcloud/Writer.cpp
    M test/unit/CMakeLists.txt
    A test/unit/drivers/pgpointcloud/PgpointcloudWriterTest.cpp
    A test/unit/drivers/pgpointcloud/Support.hpp.in

  Log Message:
  -----------
  Merge pull request #369 from gadomski/issue/292-postgres-segfault

Check for null pointers when using pg_query_once


Compare: https://github.com/PDAL/PDAL/compare/920f43d7a314...93283dd667d1


More information about the pdal-commits mailing list