From 316e9c7fafb808f6acac52b234d97929c33264ae Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 1 May 2026 13:17:46 +0000 Subject: [PATCH] Fix unsafe std::vector::data() usage that assumes nullptr for empty vectors When std::vector is empty, data() is not guaranteed to return nullptr. This caused has_times()/has_points()/etc. to incorrectly return true for empty point clouds, since those methods check pointer nullness. Fix by using `vec.empty() ? nullptr : vec.data()` pattern everywhere a storage vector's data pointer is assigned to a raw pointer field. Agent-Logs-Url: https://github.com/koide3/gtsam_points/sessions/12ae22c6-f3d9-4719-8f59-c70496ba892d Co-authored-by: koide3 <31344317+koide3@users.noreply.github.com> --- .../ann/impl/incremental_voxelmap_impl.hpp | 2 +- .../gtsam_points/types/point_cloud_cpu.hpp | 2 +- .../factors/intensity_gradients.cpp | 4 +-- .../types/gaussian_voxelmap_cpu_funcs.cpp | 6 ++-- src/gtsam_points/types/point_cloud_cpu.cpp | 34 +++++++++---------- .../types/point_cloud_cpu_funcs.cpp | 22 ++++++------ src/gtsam_points/types/point_cloud_gpu.cu | 2 +- 7 files changed, 36 insertions(+), 36 deletions(-) diff --git a/include/gtsam_points/ann/impl/incremental_voxelmap_impl.hpp b/include/gtsam_points/ann/impl/incremental_voxelmap_impl.hpp index 5faf2f08..7c75573a 100644 --- a/include/gtsam_points/ann/impl/incremental_voxelmap_impl.hpp +++ b/include/gtsam_points/ann/impl/incremental_voxelmap_impl.hpp @@ -219,7 +219,7 @@ PointCloudCPU::Ptr IncrementalVoxelMap::voxel_data() const { }); frame->num_points = frame->points_storage.size(); - frame->points = frame->points_storage.data(); + frame->points = frame->points_storage.empty() ? nullptr : frame->points_storage.data(); frame->normals = frame->normals_storage.empty() ? nullptr : frame->normals_storage.data(); frame->covs = frame->covs_storage.empty() ? nullptr : frame->covs_storage.data(); frame->intensities = frame->intensities_storage.empty() ? nullptr : frame->intensities_storage.data(); diff --git a/include/gtsam_points/types/point_cloud_cpu.hpp b/include/gtsam_points/types/point_cloud_cpu.hpp index cd03ca85..b1e2f6b9 100644 --- a/include/gtsam_points/types/point_cloud_cpu.hpp +++ b/include/gtsam_points/types/point_cloud_cpu.hpp @@ -85,7 +85,7 @@ struct PointCloudCPU : public PointCloud { void add_aux_attribute(const std::string& attrib_name, const T* values, int num_points) { auto attributes = std::make_shared>(values, values + num_points); aux_attributes_storage[attrib_name] = attributes; - aux_attributes[attrib_name] = std::make_pair(sizeof(T), attributes->data()); + aux_attributes[attrib_name] = std::make_pair(sizeof(T), attributes->empty() ? nullptr : attributes->data()); } template void add_aux_attribute(const std::string& attrib_name, const std::vector& values) { diff --git a/src/gtsam_points/factors/intensity_gradients.cpp b/src/gtsam_points/factors/intensity_gradients.cpp index 42db2966..10624ffe 100644 --- a/src/gtsam_points/factors/intensity_gradients.cpp +++ b/src/gtsam_points/factors/intensity_gradients.cpp @@ -141,12 +141,12 @@ IntensityGradients::estimate(const gtsam_points::PointCloudCPU::Ptr& frame, int if (estimate_normals) { frame->normals_storage.resize(frame->size()); - frame->normals = frame->normals_storage.data(); + frame->normals = frame->normals_storage.empty() ? nullptr : frame->normals_storage.data(); } if (estimate_covs) { frame->covs_storage.resize(frame->size()); - frame->covs = frame->covs_storage.data(); + frame->covs = frame->covs_storage.empty() ? nullptr : frame->covs_storage.data(); } IntensityGradients::Ptr gradients(new IntensityGradients); diff --git a/src/gtsam_points/types/gaussian_voxelmap_cpu_funcs.cpp b/src/gtsam_points/types/gaussian_voxelmap_cpu_funcs.cpp index 8b6c231d..8e773908 100644 --- a/src/gtsam_points/types/gaussian_voxelmap_cpu_funcs.cpp +++ b/src/gtsam_points/types/gaussian_voxelmap_cpu_funcs.cpp @@ -82,13 +82,13 @@ merge_frames(const std::vector& poses, const std::vectornum_points = num_voxels; merged->points_storage.resize(num_voxels, Eigen::Vector4d::Zero()); merged->covs_storage.resize(num_voxels, Eigen::Matrix4d::Zero()); - merged->points = merged->points_storage.data(); - merged->covs = merged->covs_storage.data(); + merged->points = merged->points_storage.empty() ? nullptr : merged->points_storage.data(); + merged->covs = merged->covs_storage.empty() ? nullptr : merged->covs_storage.data(); const bool has_intensities = std::all_of(frames.begin(), frames.end(), [](const auto& frame) { return frame->has_intensities(); }); if (has_intensities) { merged->intensities_storage.resize(num_voxels, 0.0); - merged->intensities = merged->intensities_storage.data(); + merged->intensities = merged->intensities_storage.empty() ? nullptr : merged->intensities_storage.data(); } for (int i = 0; i < frames.size(); i++) { diff --git a/src/gtsam_points/types/point_cloud_cpu.cpp b/src/gtsam_points/types/point_cloud_cpu.cpp index 30739753..d292e6bc 100644 --- a/src/gtsam_points/types/point_cloud_cpu.cpp +++ b/src/gtsam_points/types/point_cloud_cpu.cpp @@ -62,7 +62,7 @@ PointCloudCPU::Ptr PointCloudCPU::clone(const PointCloud& points) { memcpy(storage->data(), data_ptr, elem_size * points.size()); new_points->aux_attributes_storage[name] = storage; - new_points->aux_attributes[name] = std::make_pair(elem_size, storage->data()); + new_points->aux_attributes[name] = std::make_pair(elem_size, storage->empty() ? nullptr : storage->data()); } return new_points; @@ -76,7 +76,7 @@ void PointCloudCPU::add_times(const T* times, int num_points) { if (times) { std::copy(times, times + num_points, times_storage.begin()); } - this->times = this->times_storage.data(); + this->times = times_storage.empty() ? nullptr : times_storage.data(); } template void PointCloudCPU::add_times(const float* times, int num_points); @@ -91,7 +91,7 @@ void PointCloudCPU::add_points(const Eigen::Matrix* points, int num_poi points_storage[i].head() = points[i].template head().template cast(); } } - this->points = points_storage.data(); + this->points = points_storage.empty() ? nullptr : points_storage.data(); this->num_points = num_points; } @@ -110,7 +110,7 @@ void PointCloudCPU::add_normals(const Eigen::Matrix* normals, int num_p normals_storage[i].head() = normals[i].template head().template cast(); } } - this->normals = normals_storage.data(); + this->normals = normals_storage.empty() ? nullptr : normals_storage.data(); } template void PointCloudCPU::add_normals(const Eigen::Matrix* normals, int num_points); @@ -128,7 +128,7 @@ void PointCloudCPU::add_covs(const Eigen::Matrix* covs, int num_points) covs_storage[i].block(0, 0) = covs[i].template block(0, 0).template cast(); } } - this->covs = covs_storage.data(); + this->covs = covs_storage.empty() ? nullptr : covs_storage.data(); } template void PointCloudCPU::add_covs(const Eigen::Matrix* covs, int num_points); @@ -144,7 +144,7 @@ void PointCloudCPU::add_intensities(const T* intensities, int num_points) { if (intensities) { std::copy(intensities, intensities + num_points, intensities_storage.begin()); } - this->intensities = this->intensities_storage.data(); + this->intensities = intensities_storage.empty() ? nullptr : intensities_storage.data(); } template void PointCloudCPU::add_intensities(const float* intensities, int num_points); @@ -161,35 +161,35 @@ PointCloudCPU::Ptr PointCloudCPU::load(const std::string& path) { frame->num_points = num_points; frame->points_storage.resize(num_points); - frame->points = frame->points_storage.data(); + frame->points = frame->points_storage.empty() ? nullptr : frame->points_storage.data(); ifs.seekg(0, std::ios::beg); ifs.read(reinterpret_cast(frame->points), sizeof(Eigen::Vector4d) * frame->size()); if (boost::filesystem::exists(path + "/times.bin")) { frame->times_storage.resize(frame->size()); - frame->times = frame->times_storage.data(); + frame->times = frame->times_storage.empty() ? nullptr : frame->times_storage.data(); std::ifstream ifs(path + "/times.bin", std::ios::binary); ifs.read(reinterpret_cast(frame->times), sizeof(double) * frame->size()); } if (boost::filesystem::exists(path + "/normals.bin")) { frame->normals_storage.resize(frame->size()); - frame->normals = frame->normals_storage.data(); + frame->normals = frame->normals_storage.empty() ? nullptr : frame->normals_storage.data(); std::ifstream ifs(path + "/normals.bin", std::ios::binary); ifs.read(reinterpret_cast(frame->normals), sizeof(Eigen::Vector4d) * frame->size()); } if (boost::filesystem::exists(path + "/covs.bin")) { frame->covs_storage.resize(frame->size()); - frame->covs = frame->covs_storage.data(); + frame->covs = frame->covs_storage.empty() ? nullptr : frame->covs_storage.data(); std::ifstream ifs(path + "/covs.bin", std::ios::binary); ifs.read(reinterpret_cast(frame->covs), sizeof(Eigen::Matrix4d) * frame->size()); } if (boost::filesystem::exists(path + "/intensities.bin")) { frame->intensities_storage.resize(frame->size()); - frame->intensities = frame->intensities_storage.data(); + frame->intensities = frame->intensities_storage.empty() ? nullptr : frame->intensities_storage.data(); std::ifstream ifs(path + "/intensities.bin", std::ios::binary); ifs.read(reinterpret_cast(frame->intensities), sizeof(double) * frame->size()); } @@ -200,7 +200,7 @@ PointCloudCPU::Ptr PointCloudCPU::load(const std::string& path) { frame->num_points = num_points; frame->points_storage.resize(num_points); - frame->points = frame->points_storage.data(); + frame->points = frame->points_storage.empty() ? nullptr : frame->points_storage.data(); std::vector points_f(num_points); ifs.seekg(0, std::ios::beg); @@ -209,7 +209,7 @@ PointCloudCPU::Ptr PointCloudCPU::load(const std::string& path) { if (boost::filesystem::exists(path + "/times_compact.bin")) { frame->times_storage.resize(frame->size()); - frame->times = frame->times_storage.data(); + frame->times = frame->times_storage.empty() ? nullptr : frame->times_storage.data(); std::vector times_f(frame->size()); std::ifstream ifs(path + "/times_compact.bin", std::ios::binary); @@ -219,7 +219,7 @@ PointCloudCPU::Ptr PointCloudCPU::load(const std::string& path) { if (boost::filesystem::exists(path + "/normals_compact.bin")) { frame->normals_storage.resize(frame->size()); - frame->normals = frame->normals_storage.data(); + frame->normals = frame->normals_storage.empty() ? nullptr : frame->normals_storage.data(); std::vector normals_f(frame->size()); std::ifstream ifs(path + "/normals_compact.bin", std::ios::binary); @@ -231,7 +231,7 @@ PointCloudCPU::Ptr PointCloudCPU::load(const std::string& path) { if (boost::filesystem::exists(path + "/covs_compact.bin")) { frame->covs_storage.resize(frame->size()); - frame->covs = frame->covs_storage.data(); + frame->covs = frame->covs_storage.empty() ? nullptr : frame->covs_storage.data(); std::vector> covs_f(frame->size()); std::ifstream ifs(path + "/covs_compact.bin", std::ios::binary); @@ -250,7 +250,7 @@ PointCloudCPU::Ptr PointCloudCPU::load(const std::string& path) { if (boost::filesystem::exists(path + "/intensities_compact.bin")) { frame->intensities_storage.resize(frame->size()); - frame->intensities = frame->intensities_storage.data(); + frame->intensities = frame->intensities_storage.empty() ? nullptr : frame->intensities_storage.data(); std::vector intensities_f(frame->size()); std::ifstream ifs(path + "/intensities_compact.bin", std::ios::binary); @@ -287,7 +287,7 @@ PointCloudCPU::Ptr PointCloudCPU::load(const std::string& path) { ifs.read(storage->data(), bytes); frame->aux_attributes_storage[name] = storage; - frame->aux_attributes[name] = std::make_pair(elem_size, storage->data()); + frame->aux_attributes[name] = std::make_pair(elem_size, storage->empty() ? nullptr : storage->data()); } return frame; diff --git a/src/gtsam_points/types/point_cloud_cpu_funcs.cpp b/src/gtsam_points/types/point_cloud_cpu_funcs.cpp index b4926da4..a67fb787 100644 --- a/src/gtsam_points/types/point_cloud_cpu_funcs.cpp +++ b/src/gtsam_points/types/point_cloud_cpu_funcs.cpp @@ -28,30 +28,30 @@ PointCloudCPU::Ptr sample(const PointCloud::ConstPtr& frame, const std::vectornum_points = indices.size(); sampled->points_storage.resize(indices.size()); - sampled->points = sampled->points_storage.data(); + sampled->points = sampled->points_storage.empty() ? nullptr : sampled->points_storage.data(); std::transform(indices.begin(), indices.end(), sampled->points, [&](const int i) { return frame->points[i]; }); if (frame->times) { sampled->times_storage.resize(indices.size()); - sampled->times = sampled->times_storage.data(); + sampled->times = sampled->times_storage.empty() ? nullptr : sampled->times_storage.data(); std::transform(indices.begin(), indices.end(), sampled->times, [&](const int i) { return frame->times[i]; }); } if (frame->normals) { sampled->normals_storage.resize(indices.size()); - sampled->normals = sampled->normals_storage.data(); + sampled->normals = sampled->normals_storage.empty() ? nullptr : sampled->normals_storage.data(); std::transform(indices.begin(), indices.end(), sampled->normals, [&](const int i) { return frame->normals[i]; }); } if (frame->covs) { sampled->covs_storage.resize(indices.size()); - sampled->covs = sampled->covs_storage.data(); + sampled->covs = sampled->covs_storage.empty() ? nullptr : sampled->covs_storage.data(); std::transform(indices.begin(), indices.end(), sampled->covs, [&](const int i) { return frame->covs[i]; }); } if (frame->intensities) { sampled->intensities_storage.resize(indices.size()); - sampled->intensities = sampled->intensities_storage.data(); + sampled->intensities = sampled->intensities_storage.empty() ? nullptr : sampled->intensities_storage.data(); std::transform(indices.begin(), indices.end(), sampled->intensities, [&](const int i) { return frame->intensities[i]; }); } @@ -68,7 +68,7 @@ PointCloudCPU::Ptr sample(const PointCloud::ConstPtr& frame, const std::vectoraux_attributes_storage[name] = storage; - sampled->aux_attributes[name] = std::make_pair(elem_size, storage->data()); + sampled->aux_attributes[name] = std::make_pair(elem_size, storage->empty() ? nullptr : storage->data()); } return sampled; @@ -265,26 +265,26 @@ PointCloudCPU::Ptr voxelgrid_sampling(const PointCloud::ConstPtr& frame, const d downsampled->num_points = num_points; downsampled->points_storage.resize(num_points); - downsampled->points = downsampled->points_storage.data(); + downsampled->points = downsampled->points_storage.empty() ? nullptr : downsampled->points_storage.data(); if (frame->times) { downsampled->times_storage.resize(num_points); - downsampled->times = downsampled->times_storage.data(); + downsampled->times = downsampled->times_storage.empty() ? nullptr : downsampled->times_storage.data(); } if (frame->normals) { downsampled->normals_storage.resize(num_points); - downsampled->normals = downsampled->normals_storage.data(); + downsampled->normals = downsampled->normals_storage.empty() ? nullptr : downsampled->normals_storage.data(); } if (frame->covs) { downsampled->covs_storage.resize(num_points); - downsampled->covs = downsampled->covs_storage.data(); + downsampled->covs = downsampled->covs_storage.empty() ? nullptr : downsampled->covs_storage.data(); } if (frame->intensities) { downsampled->intensities_storage.resize(num_points); - downsampled->intensities = downsampled->intensities_storage.data(); + downsampled->intensities = downsampled->intensities_storage.empty() ? nullptr : downsampled->intensities_storage.data(); } if (!frame->aux_attributes.empty()) { diff --git a/src/gtsam_points/types/point_cloud_gpu.cu b/src/gtsam_points/types/point_cloud_gpu.cu index 891aed18..290971b3 100644 --- a/src/gtsam_points/types/point_cloud_gpu.cu +++ b/src/gtsam_points/types/point_cloud_gpu.cu @@ -211,7 +211,7 @@ void PointCloudGPU::download_points(CUstream_st* stream) { if (!points) { points_storage.resize(num_points); - points = points_storage.data(); + points = points_storage.empty() ? nullptr : points_storage.data(); } std::transform(points_h.begin(), points_h.end(), points, [](const Eigen::Vector3f& p) { return Eigen::Vector4d(p.x(), p.y(), p.z(), 1.0); });