Review casts in PCL, Part A [#4230] (#5504)

* Enabled cppcoreguidelines-pro-type-static-cast-downcast

* Fixed issues flagged by cppcoreguidelines-pro-type-cstyle-cast

* Fixed more issues flagged by cppcoreguidelines-pro-type-cstyle-cast

* More cppcoreguidelines-*-cast escapes

* Fixed more cppcoreguidelines-*-cast complaints

* Fixed complaint from modernize-use-auto

* Made changes per inspection

* Switched more C-style casts to reinterpret_cast

* Fixed clang-format issued flagged by CI

* Fixed C-style cast escape

* Fixed string constructor per review

Author: gnawme

.clang-tidy

@@ -1,5 +1,5 @@
 ---
-Checks: '-*,modernize-use-auto,modernize-deprecated-headers,modernize-redundant-void-arg,modernize-replace-random-shuffle,modernize-use-equals-default,modernize-use-equals-delete,modernize-use-nullptr,modernize-use-override,modernize-use-using,performance-faster-string-find,performance-for-range-copy,performance-implicit-conversion-in-loop,performance-inefficient-algorithm,performance-inefficient-vector-operation,performance-move-const-arg,performance-move-constructor-init,performance-no-automatic-move,performance-noexcept-move-constructor,performance-type-promotion-in-math-fn'
-WarningsAsErrors: '-*,modernize-use-auto,modernize-deprecated-headers,modernize-redundant-void-arg,modernize-replace-random-shuffle,modernize-use-equals-default,modernize-use-equals-delete,modernize-use-nullptr,modernize-use-override,modernize-use-using,performance-faster-string-find,performance-for-range-copy,performance-implicit-conversion-in-loop,performance-inefficient-algorithm,performance-inefficient-vector-operation,performance-move-const-arg,performance-move-constructor-init,performance-no-automatic-move,performance-noexcept-move-constructor,performance-type-promotion-in-math-fn'
+Checks: '-*,modernize-use-auto,modernize-deprecated-headers,modernize-redundant-void-arg,modernize-replace-random-shuffle,modernize-use-equals-default,modernize-use-equals-delete,modernize-use-nullptr,modernize-use-override,modernize-use-using,performance-faster-string-find,performance-for-range-copy,performance-implicit-conversion-in-loop,performance-inefficient-algorithm,performance-inefficient-vector-operation,performance-move-const-arg,performance-move-constructor-init,performance-no-automatic-move,performance-noexcept-move-constructor,performance-type-promotion-in-math-fn,cppcoreguidelines-pro-type-cstyle-cast,cppcoreguidelines-pro-type-static-cast-downcast'
+WarningsAsErrors: '-*,modernize-use-auto,modernize-deprecated-headers,modernize-redundant-void-arg,modernize-replace-random-shuffle,modernize-use-equals-default,modernize-use-equals-delete,modernize-use-nullptr,modernize-use-override,modernize-use-using,performance-faster-string-find,performance-for-range-copy,performance-implicit-conversion-in-loop,performance-inefficient-algorithm,performance-inefficient-vector-operation,performance-move-const-arg,performance-move-constructor-init,performance-no-automatic-move,performance-noexcept-move-constructor,performance-type-promotion-in-math-fn,cppcoreguidelines-pro-type-cstyle-cast,cppcoreguidelines-pro-type-static-cast-downcast'
 CheckOptions:
 - {key: modernize-use-auto.MinTypeNameLength, value: 7}

common/src/range_image_planar.cpp

@@ -210,7 +210,7 @@ namespace pcl
       std::cerr << __PRETTY_FUNCTION__<<": Given range image is not a RangeImagePlanar!\n";
       return;
     }
-    RangeImagePlanar& ret = * (static_cast<RangeImagePlanar*> (&half_image));
+    RangeImagePlanar& ret = * (dynamic_cast<RangeImagePlanar*> (&half_image));
 
     ret.focal_length_x_ = focal_length_x_/2;
     ret.focal_length_x_reciprocal_ = 1.0f/ret.focal_length_x_;
@@ -233,7 +233,7 @@ namespace pcl
       std::cerr << __PRETTY_FUNCTION__<<": Given range image is not a RangeImagePlanar!\n";
       return;
     }
-    RangeImagePlanar& ret = * (static_cast<RangeImagePlanar*> (&sub_image));
+    RangeImagePlanar& ret = * (dynamic_cast<RangeImagePlanar*> (&sub_image));
 
     ret.focal_length_x_ = focal_length_x_ / static_cast<float> (combine_pixels);
     ret.focal_length_x_reciprocal_ = 1.0f / ret.focal_length_x_;
@@ -255,7 +255,7 @@ namespace pcl
       std::cerr << PVARC(typeid (*this).name())<<PVARN(typeid (other).name());
     }
     assert (ERROR_GIVEN_RANGE_IMAGE_IS_NOT_A_RangeImagePlanar);
-    *static_cast<RangeImagePlanar*> (&other) = *this;
+    *dynamic_cast<RangeImagePlanar*> (&other) = *this;
   }
 
 }  // namespace end

io/src/ifs_io.cpp

@@ -78,7 +78,7 @@ pcl::IFSReader::readHeader (const std::string &file_name, pcl::PCLPointCloud2 &c
 
   //Read the magic
   std::uint32_t length_of_magic;
-  fs.read ((char*)&length_of_magic, sizeof (std::uint32_t));
+  fs.read (reinterpret_cast<char*>(&length_of_magic), sizeof (std::uint32_t));
   char *magic = new char [length_of_magic];
   fs.read (magic, sizeof (char) * length_of_magic);
   const bool file_is_ifs_file = (strcmp (magic, "IFS") == 0);
@@ -92,7 +92,7 @@ pcl::IFSReader::readHeader (const std::string &file_name, pcl::PCLPointCloud2 &c
 
   //Read IFS version
   float version;
-  fs.read ((char*)&version, sizeof (float));
+  fs.read (reinterpret_cast<char*>(&version), sizeof (float));
   if (version == 1.0f)
     ifs_version = IFS_V1_0;
   else
@@ -107,7 +107,7 @@ pcl::IFSReader::readHeader (const std::string &file_name, pcl::PCLPointCloud2 &c
 
   //Read the name
   std::uint32_t length_of_name;
-  fs.read ((char*)&length_of_name, sizeof (std::uint32_t));
+  fs.read (reinterpret_cast<char*>(&length_of_name), sizeof (std::uint32_t));
   char *name = new char [length_of_name];
   fs.read (name, sizeof (char) * length_of_name);
   delete[] name;
@@ -119,15 +119,15 @@ pcl::IFSReader::readHeader (const std::string &file_name, pcl::PCLPointCloud2 &c
     {
       //Read the keyword
       std::uint32_t length_of_keyword;
-      fs.read ((char*)&length_of_keyword, sizeof (std::uint32_t));
+      fs.read (reinterpret_cast<char*>(&length_of_keyword), sizeof (std::uint32_t));
       char *keyword = new char [length_of_keyword];
       fs.read (keyword, sizeof (char) * length_of_keyword);
 
       const bool keyword_is_vertices = (strcmp (keyword, "VERTICES") == 0);
       delete[] keyword;
       if (keyword_is_vertices)
       {
-        fs.read ((char*)&nr_points, sizeof (std::uint32_t));
+        fs.read (reinterpret_cast<char*>(&nr_points), sizeof (std::uint32_t));
         if ((nr_points == 0) || (nr_points > 10000000))
         {
           PCL_ERROR ("[pcl::IFSReader::readHeader] Bad number of vertices %lu!\n", nr_points);
@@ -281,7 +281,7 @@ pcl::IFSReader::read (const std::string &file_name, pcl::PolygonMesh &mesh, int
   fs.seekg (data_size);
   // Read the TRIANGLES keyword
   std::uint32_t length_of_keyword;
-  fs.read ((char*)&length_of_keyword, sizeof (std::uint32_t));
+  fs.read (reinterpret_cast<char*>(&length_of_keyword), sizeof (std::uint32_t));
   char *keyword = new char [length_of_keyword];
   fs.read (keyword, sizeof (char) * length_of_keyword);
   if (strcmp (keyword, "TRIANGLES"))
@@ -293,7 +293,7 @@ pcl::IFSReader::read (const std::string &file_name, pcl::PolygonMesh &mesh, int
   delete[] keyword;
   // Read the number of facets
   std::uint32_t nr_facets;
-  fs.read ((char*)&nr_facets, sizeof (std::uint32_t));
+  fs.read (reinterpret_cast<char*>(&nr_facets), sizeof (std::uint32_t));
   if ((nr_facets == 0) || (nr_facets > 10000000))
   {
     PCL_ERROR ("[pcl::IFSReader::read] Bad number of facets %lu!\n", nr_facets);
@@ -307,9 +307,9 @@ pcl::IFSReader::read (const std::string &file_name, pcl::PolygonMesh &mesh, int
   {
     pcl::Vertices &facet = mesh.polygons[i];
     facet.vertices.resize (3);
-    fs.read ((char*)&(facet.vertices[0]), sizeof (std::uint32_t));
-    fs.read ((char*)&(facet.vertices[1]), sizeof (std::uint32_t));
-    fs.read ((char*)&(facet.vertices[2]), sizeof (std::uint32_t));
+    fs.read (reinterpret_cast<char*>(&(facet.vertices[0])), sizeof (std::uint32_t));
+    fs.read (reinterpret_cast<char*>(&(facet.vertices[1])), sizeof (std::uint32_t));
+    fs.read (reinterpret_cast<char*>(&(facet.vertices[2])), sizeof (std::uint32_t));
   }
   // We are done, close the file
   fs.close ();

io/src/image_rgb24.cpp

@@ -81,7 +81,7 @@ pcl::io::ImageRGB24::fillGrayscale (unsigned width, unsigned height, unsigned ch
     unsigned dst_skip = gray_line_step - width; // skip of padding values in bytes
 
     unsigned char* dst_line = gray_buffer;
-    const RGB888Pixel* src_line = (RGB888Pixel*) wrapper_->getData ();
+    const auto* src_line = reinterpret_cast<const RGB888Pixel*>(wrapper_->getData ());
 
     for (unsigned yIdx = 0; yIdx < height; ++yIdx, src_line += src_skip, dst_line += dst_skip)
     {

io/src/image_yuv422.cpp

@@ -79,7 +79,7 @@ pcl::io::ImageYUV422::fillRGB (unsigned width, unsigned height, unsigned char* r
       THROW_IO_EXCEPTION ("Downsampling only possible for power of two scale in both dimensions. Request was %d x %d -> %d x %d.", wrapper_->getWidth (), wrapper_->getHeight (), width, height);
   }
 
-  const std::uint8_t* yuv_buffer = (std::uint8_t*) wrapper_->getData ();
+  const auto* yuv_buffer = reinterpret_cast<const std::uint8_t*>(wrapper_->getData ());
 
   unsigned rgb_line_skip = 0;
   if (rgb_line_step != 0)
@@ -143,7 +143,7 @@ pcl::io::ImageYUV422::fillGrayscale (unsigned width, unsigned height, unsigned c
   unsigned yuv_step = wrapper_->getWidth () / width;
   unsigned yuv_x_step = yuv_step << 1;
   unsigned yuv_skip = (wrapper_->getHeight () / height - 1) * ( wrapper_->getWidth () << 1 );
-  const std::uint8_t* yuv_buffer = ( (std::uint8_t*) wrapper_->getData () + 1);
+  const std::uint8_t* yuv_buffer = ( reinterpret_cast<const std::uint8_t*>(wrapper_->getData ()) + 1);
 
   for (unsigned yIdx = 0; yIdx < wrapper_->getHeight (); yIdx += yuv_step, yuv_buffer += yuv_skip, gray_buffer += gray_line_skip)
   {

io/src/robot_eye_grabber.cpp

@@ -39,6 +39,8 @@
 #include <pcl/common/point_tests.h> // for pcl::isFinite
 #include <pcl/console/print.h>
 
+#include <string>
+
 /////////////////////////////////////////////////////////////////////////////
 pcl::RobotEyeGrabber::RobotEyeGrabber ()
   : terminate_thread_ (false)
@@ -176,13 +178,15 @@ pcl::RobotEyeGrabber::convertPacketData (unsigned char *data_packet, std::size_t
     //The new packet data format contains this as a header
     //char[6]  "EBRBEP"
     //std::uint32_t Timestamp // counts of a 66 MHz clock since power-on of eye.
-    std::size_t response_size = 6; //"EBRBEP"
-    if( !strncmp((char*)(data_packet), "EBRBEP", response_size) )
+    const std::string PACKET_HEADER("EBRBEP");
+    constexpr std::size_t RESPONSE_SIZE = 6; //"EBRBEP"
+    std::string packet(reinterpret_cast<const char*>(data_packet), RESPONSE_SIZE);
+    if(packet == PACKET_HEADER)
     {
       std::uint32_t timestamp; // counts of a 66 MHz clock since power-on of eye.
-      computeTimestamp(timestamp, data_packet + response_size);
+      computeTimestamp(timestamp, data_packet + RESPONSE_SIZE);
       //std::cout << "Timestamp: " << timestamp << std::endl;
-      offset = (response_size + sizeof(timestamp));
+      offset = (RESPONSE_SIZE + sizeof(timestamp));
     }
     else
     {

ml/src/svm.cpp

@@ -370,7 +370,7 @@ class Kernel : public pcl::QMatrix {
   double
   kernel_precomputed(int i, int j) const
   {
-    return x[i][int(x[j][0].value)].value;
+    return x[i][static_cast<int>(x[j][0].value)].value;
   }
 };
 
@@ -493,7 +493,7 @@ Kernel::k_function(const svm_node* x, const svm_node* y, const svm_parameter& pa
     return std::tanh(param.gamma * dot(x, y) + param.coef0);
 
   case PRECOMPUTED: // x: test (validation), y: SV
-    return x[int(y->value)].value;
+    return x[static_cast<int>(y->value)].value;
 
   default:
     return 0; // Unreachable
@@ -1461,7 +1461,7 @@ class SVC_Q : public Kernel {
 
     if ((start = cache->get_data(i, &data, len)) < len) {
       for (int j = start; j < len; j++)
-        data[j] = Qfloat(y[i] * y[j] * (this->*kernel_function)(i, j));
+        data[j] = static_cast<Qfloat>(y[i] * y[j] * (this->*kernel_function)(i, j));
     }
 
     return data;
@@ -1516,7 +1516,7 @@ class ONE_CLASS_Q : public Kernel {
 
     if ((start = cache->get_data(i, &data, len)) < len) {
       for (int j = start; j < len; j++)
-        data[j] = Qfloat((this->*kernel_function)(i, j));
+        data[j] = static_cast<Qfloat>((this->*kernel_function)(i, j));
     }
 
     return data;
@@ -1590,7 +1590,7 @@ class SVR_Q : public Kernel {
 
     if (cache->get_data(real_i, &data, l) < l) {
       for (j = 0; j < l; j++)
-        data[j] = Qfloat((this->*kernel_function)(real_i, j));
+        data[j] = static_cast<Qfloat>((this->*kernel_function)(real_i, j));
     }
 
     // reorder and copy
@@ -1601,7 +1601,7 @@ class SVR_Q : public Kernel {
     schar si = sign[i];
 
     for (j = 0; j < len; j++)
-      buf[j] = Qfloat(si) * Qfloat(sign[j]) * data[index[j]];
+      buf[j] = static_cast<Qfloat>(si) * static_cast<Qfloat>(sign[j]) * data[index[j]];
 
     return buf;
   }
@@ -1765,7 +1765,7 @@ solve_one_class(const svm_problem* prob,
   double* zeros = new double[l];
   schar* ones = new schar[l];
 
-  int n = int(param->nu * prob->l); // # of alpha's at upper bound
+  int n = static_cast<int>(param->nu * prob->l); // # of alpha's at upper bound
 
   for (int i = 0; i < n; i++)
     alpha[i] = 1;
@@ -2340,7 +2340,7 @@ svm_group_classes(const svm_problem* prob,
   int* data_label = Malloc(int, l);
 
   for (int i = 0; i < l; i++) {
-    int this_label = int(prob->y[i]);
+    int this_label = static_cast<int>(prob->y[i]);
     int j;
 
     for (j = 0; j < nr_class; j++) {
@@ -3097,7 +3097,7 @@ svm_save_model(const char* model_file_name, const svm_model* model)
     const svm_node* p = SV[i];
 
     if (param.kernel_type == PRECOMPUTED)
-      fprintf(fp, "0:%d ", int(p->value));
+      fprintf(fp, "0:%d ", static_cast<int>(p->value));
     else
       while (p->index != -1) {
         fprintf(fp, "%d:%.8g ", p->index, p->value);
@@ -3124,7 +3124,7 @@ readline(FILE* input)
   while (strrchr(line, '\n') == nullptr) {
     max_line_len *= 2;
     line = static_cast<char*>(realloc(line, max_line_len));
-    int len = int(strlen(line));
+    int len = static_cast<int>(strlen(line));
 
     if (fgets(line + len, max_line_len - len, input) == nullptr)
       break;
@@ -3374,7 +3374,7 @@ svm_load_model(const char* model_file_name)
       if (val == nullptr)
         break;
 
-      x_space[j].index = int(strtol(idx, nullptr, 10));
+      x_space[j].index = static_cast<int>(strtol(idx, nullptr, 10));
 
       x_space[j].value = strtod(val, nullptr);
 
@@ -3521,7 +3521,7 @@ svm_check_parameter(const svm_problem* prob, const svm_parameter* param)
     int* count = Malloc(int, max_nr_class);
 
     for (int i = 0; i < l; i++) {
-      int this_label = int(prob->y[i]);
+      int this_label = static_cast<int>(prob->y[i]);
       int j;
 
       for (j = 0; j < nr_class; j++)

ml/src/svm_wrapper.cpp

@@ -253,8 +253,7 @@ pcl::SVMTrain::trainClassifier()
     doCrossValidation();
   }
   else {
-    SVMModel* out;
-    out = static_cast<SVMModel*>(svm_train(&prob_, &param_));
+    auto* out = reinterpret_cast<SVMModel*>(svm_train(&prob_, &param_));
     if (out == nullptr) {
       PCL_ERROR("[pcl::%s::trainClassifier] Error taining the classifier model.\n",
                 getClassName().c_str());
@@ -499,8 +498,7 @@ pcl::SVM::saveProblemNorm(const char* filename,
 bool
 pcl::SVMClassify::loadClassifierModel(const char* filename)
 {
-  SVMModel* out;
-  out = static_cast<SVMModel*>(svm_load_model(filename));
+  auto* out = reinterpret_cast<SVMModel*>(svm_load_model(filename));
   if (out == nullptr) {
     PCL_ERROR("[pcl::%s::loadClassifierModel] Can't open classifier model %s.\n",
               getClassName().c_str(),

outofcore/tools/outofcore_viewer.cpp

@@ -159,7 +159,7 @@ class KeyboardCallback : public vtkCommand
     std::cout << "Key Pressed: " << key << std::endl;
 
     Scene *scene = Scene::instance ();
-    OutofcoreCloud *cloud = static_cast<OutofcoreCloud*> (scene->getObjectByName ("my_octree"));
+    OutofcoreCloud *cloud = dynamic_cast<OutofcoreCloud*> (scene->getObjectByName ("my_octree"));
 
     if (key == "Up" || key == "Down")
     {

people/apps/main_ground_based_people_detection.cpp

@@ -101,7 +101,7 @@ struct callback_args{
 void
 pp_callback (const pcl::visualization::PointPickingEvent& event, void* args)
 {
-  auto* data = (struct callback_args *)args;
+  auto* data = reinterpret_cast<struct callback_args *>(args);
   if (event.getPointIndex () == -1)
     return;
   PointT current_point;