Suppress control reaches end of non-void function in io.h

[SergioRAgostinho]

There were some indentation changes (I shouldn't I know...) because of a previous use of a tab instead of spaces. It was looking very bad with my editor.

[SergioRAgostinho]

We're testing a field _ encoded as a byte sized floating point variable which was responsible for the test failure.

pcl/test/io/test_io.cpp

Lines 57 to 108 in 24abc41

	//////////////////////////////////////////////////////////////////////////////////////////////////////////////////
	TEST (PCL, ComplexPCDFileASCII)
	{
	std::ofstream fs;
	fs.open ("complex_ascii.pcd");
	fs << "# .PCD v0.7 - Point Cloud Data file format\n"
	"VERSION 0.7\n"
	"FIELDS fpfh _ x y z\n"
	"SIZE 4 1 4 4 4\n"
	"TYPE F F F F F\n"
	"COUNT 33 10 1 1 1\n"
	"WIDTH 1\n"
	"HEIGHT 1\n"
	"VIEWPOINT 0 0 0 1 0 0 0\n"
	"POINTS 1\n"
	"DATA ascii\n"
	"0 0 0 0 0 100 0 0 0 0 0 0 0 0 0 0 100 0 0 0 0 0 0 0 0 0 0 100 0 0 0 0 0 1 1 1 1 1 1 1 1 1 1 -69.234001 -65.460999 19.173";
	fs.close ();
	pcl::PCLPointCloud2 blob;
	int res = loadPCDFile ("complex_ascii.pcd", blob);
	EXPECT_NE (int (res), -1);
	EXPECT_EQ (blob.width, 1);
	EXPECT_EQ (blob.height, 1);
	EXPECT_EQ (blob.is_dense, true);
	EXPECT_EQ (blob.data.size (), 4 * 33 + 10 * 1 + 4 * 3);
	// Check fields
	EXPECT_EQ (blob.fields[0].name, "fpfh");
	EXPECT_EQ (blob.fields[0].offset, 0);
	EXPECT_EQ (blob.fields[0].count, 33);
	EXPECT_EQ (blob.fields[0].datatype, pcl::PCLPointField::FLOAT32);
	EXPECT_EQ (blob.fields[1].name, "_");
	EXPECT_EQ (blob.fields[1].offset, 4 * 33);
	EXPECT_EQ (blob.fields[1].count, 10);
	EXPECT_EQ (blob.fields[1].datatype, pcl::PCLPointField::FLOAT32);
	EXPECT_EQ (blob.fields[2].name, "x");
	EXPECT_EQ (blob.fields[2].offset, 4 * 33 + 10 * 1);
	EXPECT_EQ (blob.fields[2].count, 1);
	EXPECT_EQ (blob.fields[2].datatype, pcl::PCLPointField::FLOAT32);
	EXPECT_EQ (blob.fields[3].name, "y");
	EXPECT_EQ (blob.fields[3].offset, 4 * 33 + 10 * 1 + 4);
	EXPECT_EQ (blob.fields[3].count, 1);
	EXPECT_EQ (blob.fields[3].datatype, pcl::PCLPointField::FLOAT32);
	EXPECT_EQ (blob.fields[4].name, "z");
	EXPECT_EQ (blob.fields[4].offset, 4 * 33 + 10 * 1 + 4 + 4);
	EXPECT_EQ (blob.fields[4].count, 1);
	EXPECT_EQ (blob.fields[4].datatype, pcl::PCLPointField::FLOAT32);

In modern day desktops there's not really a type for a byte sized float. I'm gonna cast up to float32 every time the size is equal or inferior to 4bytes.

[taketwo]

Sergio, can you provide a bit more context? Are you solving some issue that has been identified before? Or regression?

[SergioRAgostinho]

The story so far:

• The first commit was to suppress a warning of "control reaching end of non void function" which started popping after a recent PR I merged.
• Then I noticed one unit test started failing, in particular this one I pasted in my previous comment, the reason being a field which is declared F (float) but with size 1 byte. What failed was the assertion in line 93, in which that 1 byte Float field is supposed to be stored in PCLPointField::FLOAT32.
• This made me realize that in PCDs, there can be Float fields with a size inferior than 4 bytes, and that was not being taken into account in the logic that was implemented before.

[taketwo]

Thanks for the explanation. This is a truly curious find!
But what are the implications of returning FLOAT32 for a field that is said to occupy 2 bytes? How will it be read from the stream then?

[SergioRAgostinho]

Oh... true... this conversion is not handled by the compiler. So after digging a little bit more I found out that fields with the name _ are used for padding (hence the size inferior to 4 bytes) and ignored, so in theory I can return that it is an invalid type. The expected value in the unit test should be changed then.

[taketwo]

We can also introduce new types like PAD8, PAD16, etc. This may be an overkill though.

[SergioRAgostinho]

I don't think we would gain much from it though.

[taketwo]

OK, then let's stay with -1.