Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[io.cpp] PCLPointCloud2 size is limited by point_offset variable #6210

Closed
Nosille opened this issue Jan 3, 2025 · 4 comments
Closed

[io.cpp] PCLPointCloud2 size is limited by point_offset variable #6210

Nosille opened this issue Jan 3, 2025 · 4 comments
Labels

Comments

@Nosille
Copy link

Nosille commented Jan 3, 2025

Unlike other index variables in io.cpp, point_offset is simply defined as an int (see line 178). As a result, I am experiencing crashes when concatenating large pointclouds (points=3,000,000,000) with a large number of fields (15 floats). It crashes on the memcpy on line 192.

At a minimum, I think it should be changed to std::uint32_t, consistent with the point_step variable which seems to be enough to fix my current issue. However, the point_offset value is currently used in a manner where the maximum value is width * height * point_step which can result point_offset values that significantly exceed the other terms. A larger datatype, or a rework of the loop, seems worth considering to prevent this type of crash in the future.

@Nosille Nosille added kind: bug Type of issue status: triage Labels incomplete labels Jan 3, 2025
@mvieth mvieth added module: common and removed status: triage Labels incomplete labels Jan 4, 2025
@mvieth
Copy link
Member

mvieth commented Jan 4, 2025

@Nosille Hi, which version of the PCL code are you referring to? Because on the master branch, point_offset is now a std::size_t: https://github.com/PointCloudLibrary/pcl/blob/master/common/src/io.cpp#L180
The change was made by this commit: 3b44145

@Nosille
Copy link
Author

Nosille commented Jan 8, 2025

I am using 1.13 and checked 1.14.1 before created this issue, but I did not check master.

@mvieth
Copy link
Member

mvieth commented Jan 9, 2025

@Nosille Yes, it is a rather recent change. So I guess this issue can be closed, since your problem would not occur on master branch any more?

@mvieth
Copy link
Member

mvieth commented Jan 15, 2025

Closing because it is fixed on master ...

@mvieth mvieth closed this as completed Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants