search

ros-perception / laser_filters

Assorted filters designed to operate on 2D planar laser scanners, which use the sensor_msgs/LaserScan type.
BSD 3-Clause "New" or "Revised" License
166 stars 204 forks source link

Intensity Filter Breaks with Big Freaking intensities #8

Closed DLu closed 11 years ago

DLu commented 11 years ago

If intensities in a laser scan are large enough, intensity filter will segfault.

Given an intensity of 3e20 (for example), so this line results in an integer so large that it overflows. However the negative index results in the segfault here

Corrected version:

  int cur_bucket = (int) ((filtered_scan.intensities[i]/hist_max)*num_buckets) ;
  if (cur_bucket >= num_buckets-1 || cur_bucket < 0)
    cur_bucket = num_buckets-1 ;

  histogram[cur_bucket]++ ;
mintar commented 11 years ago

Hi David, I've chosen a slightly different approach in my pull request, so we don't have to rely on the integer overflow producing a negative number.

DLu commented 11 years ago

Good call. Thanks!

mintar commented 11 years ago

Wow, that was quick! :-)

vrabaud commented 11 years ago

Sorry for the delay, GSOC keeps us busy these days :) The histogram is actually for debug purposes only. @mintar patch was modifying the filtered_scan. Is that a desired behavior ? I did that instead: 2da0e525b1bd2c58a5269901eb57325fece1cd9e

Please review and close the bug appropriately.

mintar commented 11 years ago

@vrabaud Thanks, this works. I didn't care about modifying the filtered_scan, because the relevant points are removed anyway by the line before (by setting the range to max + 1), so the intensities that I modified should be ignored anyway. That behaviour isn't "desired", it shouldn't matter one way or the other.

I don't have the permissions to close this issue, but I just verified it works now, so feel free to close the issue.

vrabaud commented 11 years ago

cool, thx for the feedback !