Simplify Clumplet Reader/Writer + code analysis issues

builds/win32/msvc15/common_test.vcxproj

@@ -176,6 +176,8 @@
     </ResourceCompile>
   </ItemGroup>
   <ItemGroup>
+    <ClCompile Include="..\..\..\src\common\classes\tests\ClumpletTest.cpp" />
+    <ClCompile Include="..\..\..\src\common\classes\tests\VectorTest.cpp" />
     <ClCompile Include="..\..\..\src\common\tests\CommonTest.cpp" />
     <ClCompile Include="..\..\..\src\common\tests\CvtTest.cpp" />
     <ClCompile Include="..\..\..\src\common\tests\StringTest.cpp" />

builds/win32/msvc15/common_test.vcxproj.filters

@@ -36,5 +36,11 @@
     <ClCompile Include="..\..\..\src\yvalve\gds.cpp">
       <Filter>source</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\..\src\common\classes\tests\ClumpletTest.cpp">
+      <Filter>source</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\..\src\common\classes\tests\VectorTest.cpp">
+      <Filter>source</Filter>
+    </ClCompile>
   </ItemGroup>
 </Project>

src/common/classes/ClumpletReader.cpp

@@ -62,11 +62,11 @@ class ClumpletDump : public ClumpletReader
 		return t2;
 	}
 protected:
-	virtual void usage_mistake(const char* what) const
+	[[noreturn]] virtual void usage_mistake(const char* what) const
 	{
 		fatal_exception::raiseFmt("Internal error when using clumplet API: %s", what);
 	}
-	virtual void invalid_structure(const char* what, const int data) const
+	[[noreturn]] virtual void invalid_structure(const char* what, const int data) const
 	{
 		fatal_exception::raiseFmt("Invalid clumplet buffer structure: %s (%d)", what, data);
 	}
@@ -86,8 +86,7 @@ void ClumpletReader::dump() const
 
 	try {
 		ClumpletDump d(kind, getBuffer(), getBufferLength());
-		int t = (kind == SpbStart || kind == UnTagged || kind == WideUnTagged || kind == SpbResponse || kind == InfoResponse) ?
-			-1 : d.getBufferTag();
+		const int t = isTagged() ? d.getBufferTag() : -1;
 		gds__log("Tag=%d Offset=%d Length=%d Eof=%d\n", t, getCurOffset(), getBufferLength(), isEof());
 		for (d.rewind(); !(d.isEof()); d.moveNext())
 		{
@@ -98,7 +97,7 @@ void ClumpletReader::dump() const
 	catch (const fatal_exception& x)
 	{
 		gds__log("Fatal exception during clumplet dump: %s", x.what());
-		FB_SIZE_T l = getBufferLength() - getCurOffset();
+		const FB_SIZE_T l = getBufferLength() - getCurOffset();
 		const UCHAR *p = getBuffer() + getCurOffset();
 		gds__log("Plain dump starting with offset %d: %s", getCurOffset(),
 			ClumpletDump::hexString(p, l).c_str());
@@ -205,7 +204,7 @@ void ClumpletReader::invalid_structure(const char* what, const int data) const
 	fatal_exception::raiseFmt("Invalid clumplet buffer structure: %s (%d)", what, data);
 }
 
-bool ClumpletReader::isTagged() const
+bool ClumpletReader::isTagged() const noexcept
 {
 	switch (kind)
 	{
@@ -214,13 +213,17 @@ bool ClumpletReader::isTagged() const
 	case WideTagged:
 	case SpbAttach:
 		return true;
+	default:
+		return false;
 	}
-
-	return false;
 }
 
 UCHAR ClumpletReader::getBufferTag() const
 {
+	if (!isTagged()) {
+		usage_mistake("buffer is not tagged");
+		return 0;
+	}
 	const UCHAR* const buffer_end = getBufferEnd();
 	const UCHAR* buffer_start = getBuffer();
 
@@ -235,16 +238,6 @@ UCHAR ClumpletReader::getBufferTag() const
 			return 0;
 		}
 		return buffer_start[0];
-	case SpbStart:
-	case UnTagged:
-	case WideUnTagged:
-	case SpbSendItems:
-	case SpbReceiveItems:
-	case SpbResponse:
-	case InfoResponse:
-	case InfoItems:
-		usage_mistake("buffer is not tagged");
-		return 0;
 	case SpbAttach:
 		if (buffer_end - buffer_start == 0)
 		{
@@ -286,6 +279,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
 		case isc_tpb_lock_timeout:
 		case isc_tpb_at_snapshot_number:
 			return TraditionalDpb;
+		default:
+			break;
 		}
 		return SingleTpb;
 	case SpbSendItems:
@@ -298,6 +293,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
 		case isc_info_length:
 		case isc_info_flag_end:
 			return SingleTpb;
+		default:
+			break;
 		}
 		return StringSpb;
 	case SpbReceiveItems:
@@ -311,6 +308,9 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
 		case isc_spb_auth_plugin_name:
 		case isc_spb_auth_plugin_list:
 			return Wide;
+		default:
+			// continues with spbState below
+			break;
 		}
 		switch (spbState)
 		{
@@ -345,6 +345,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
 			case isc_spb_res_access_mode:
 			case isc_spb_res_replica_mode:
 				return ByteSpb;
+			default:
+				break;
 			}
 			invalid_structure("unknown parameter for backup/restore", tag);
 			break;
@@ -363,6 +365,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
 			case isc_spb_rpr_rollback_trans_64:
 			case isc_spb_rpr_recover_two_phase_64:
 				return BigIntSpb;
+			default:
+				break;
 			}
 			invalid_structure("unknown parameter for repair", tag);
 			break;
@@ -388,6 +392,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
 			case isc_spb_sec_groupid:
 			case isc_spb_sec_admin:
 				return IntSpb;
+			default:
+				break;
 			}
 			invalid_structure("unknown parameter for security database operation", tag);
 			break;
@@ -414,6 +420,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
 			case isc_spb_prp_online_mode:
 			case isc_spb_prp_replica_mode:
 				return ByteSpb;
+			default:
+				break;
 			}
 			invalid_structure("unknown parameter for setting database properties", tag);
 			break;
@@ -428,6 +436,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
 				return StringSpb;
 			case isc_spb_options:
 				return IntSpb;
+			default:
+				break;
 			}
 			invalid_structure("unknown parameter for getting statistics", tag);
 			break;
@@ -450,6 +460,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
 				return IntSpb;
 			case isc_spb_nbk_clean_history:
 				return SingleTpb;
+			default:
+				break;
 			}
 			invalid_structure("unknown parameter for nbackup", tag);
 			break;
@@ -460,6 +472,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
 				return StringSpb;
 			case isc_spb_options:
 				return IntSpb;
+			default:
+				break;
 			}
 			invalid_structure("unknown parameter for nbackup", tag);
 			break;
@@ -474,7 +488,10 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
 				return StringSpb;
 			case isc_spb_trc_id:
 				return IntSpb;
+			default:
+				break;
 			}
+			invalid_structure("unknown parameter for trace", tag);
 			break;
 		case isc_action_svc_validate:
 			switch (tag)
@@ -487,7 +504,12 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
 				return StringSpb;
 			case isc_spb_val_lock_timeout:
 				return IntSpb;
+			default:
+				break;
 			}
+			invalid_structure("unknown parameter for validate", tag);
+			break;
+		default:
 			break;
 		}
 		invalid_structure("wrong spb state", spbState);
@@ -533,6 +555,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
 		case isc_spb_tra_state:
 		case isc_spb_tra_advise:
 			return ByteSpb;
+		default:
+			break;
 		}
 		invalid_structure("unrecognized service response tag", tag);
 		break;
@@ -543,26 +567,24 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
 		case isc_info_truncated:
 		case isc_info_flag_end:
 			return SingleTpb;
+		default:
+			break;
 		}
 		return StringSpb;
+	default:
+		break;
 	}
 	invalid_structure("unknown clumplet kind", kind);
 	return SingleTpb;
 }
 
 void ClumpletReader::adjustSpbState()
 {
-	switch (kind)
+	if (kind == SpbStart && 
+		spbState == 0 &&							// Just started with service start block ...
+		getClumpletSize(true, true, true) == 1)		// and this is action_XXX clumplet
 	{
-	case SpbStart:
-		if (spbState == 0 &&							// Just started with service start block ...
-			getClumpletSize(true, true, true) == 1)		// and this is action_XXX clumplet
-		{
-			spbState = getClumpTag();
-		}
-		break;
-	default:
-		break;
+		spbState = getClumpTag();
 	}
 }
 
@@ -582,7 +604,7 @@ FB_SIZE_T ClumpletReader::getClumpletSize(bool wTag, bool wLength, bool wData) c
 	FB_SIZE_T lengthSize = 0;
 	FB_SIZE_T dataSize = 0;
 
-	ClumpletType t = getClumpletType(clumplet[0]);
+	const ClumpletType t = getClumpletType(clumplet[0]);
 	switch (t)
 	{
 
@@ -651,13 +673,14 @@ FB_SIZE_T ClumpletReader::getClumpletSize(bool wTag, bool wLength, bool wData) c
 
 	default:
 		invalid_structure("unknown clumplet type", t);
+		return rc;
 	}
 
 	const FB_SIZE_T total = 1 + lengthSize + dataSize;
 	if (clumplet + total > buffer_end)
 	{
 		invalid_structure("buffer end before end of clumplet - clumplet too long", total);
-		FB_SIZE_T delta = total - (buffer_end - clumplet);
+		const FB_SIZE_T delta = total - (buffer_end - clumplet);
 		if (delta > dataSize)
 			dataSize = 0;
 		else
@@ -678,50 +701,33 @@ void ClumpletReader::moveNext()
 	if (isEof())
 		return;		// no need to raise useless exceptions
 
-	switch (kind)
+	if (kind == InfoResponse)
 	{
-	case InfoResponse:
 		switch (getClumpTag())
 		{
 		case isc_info_end:
 		case isc_info_truncated:
 			// terminating clumplet
 			cur_offset = getBufferLength();
 			return;
+		default:
+			break;
 		}
 	}
 
-	FB_SIZE_T cs = getClumpletSize(true, true, true);
+	const FB_SIZE_T cs = getClumpletSize(true, true, true);
 	adjustSpbState();
 	cur_offset += cs;
 }
 
 void ClumpletReader::rewind()
 {
-	if (! getBuffer())
-	{
+	if (!getBuffer() || !isTagged())
 		cur_offset = 0;
-		spbState = 0;
-		return;
-	}
-	switch (kind)
-	{
-	case UnTagged:
-	case WideUnTagged:
-	case SpbStart:
-	case SpbSendItems:
-	case SpbReceiveItems:
-	case SpbResponse:
-	case InfoResponse:
-	case InfoItems:
-		cur_offset = 0;
-		break;
-	default:
-		if (kind == SpbAttach && getBufferLength() > 0 && getBuffer()[0] != isc_spb_version1)
-			cur_offset = 2;
-		else
-			cur_offset = 1;
-	}
+	else if (kind == SpbAttach && getBufferLength() > 0 && getBuffer()[0] == isc_spb_version)
+		cur_offset = 2;
+	else
+		cur_offset = 1;
 	spbState = 0;
 }
 
@@ -956,7 +962,7 @@ AuthReader::AuthReader(MemoryPool& pool, const AuthBlock& authBlock)
 	rewind();
 }
 
-static inline void erase(NoCaseString& s)
+static inline void erase(NoCaseString& s) noexcept
 {
 	s.erase();
 }