Skip to content

Commit c74ca39

Browse files
mrotteveelmrotteveel
authored
Simplify Clumplet Reader/Writer + code analysis issues (#8507)
* Add missing tests to Visual Studio * Remove different ways of discerning tagged/untagged; use isTagged for correct options * get tag before clearing buffer * Improve [[noreturn]] and noexcept annotations * Init kindList explicitly * Remove nested switch that doesn't make sense for SpbStart
1 parent 90e0f49 commit c74ca39

File tree

8 files changed

+122
-117
lines changed

8 files changed

+122
-117
lines changed

‎builds/win32/msvc15/common_test.vcxproj‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,8 @@
176176
</ResourceCompile>
177177
</ItemGroup>
178178
<ItemGroup>
179+
<ClCompile Include="..\..\..\src\common\classes\tests\ClumpletTest.cpp" />
180+
<ClCompile Include="..\..\..\src\common\classes\tests\VectorTest.cpp" />
179181
<ClCompile Include="..\..\..\src\common\tests\CommonTest.cpp" />
180182
<ClCompile Include="..\..\..\src\common\tests\CvtTest.cpp" />
181183
<ClCompile Include="..\..\..\src\common\tests\StringTest.cpp" />

‎builds/win32/msvc15/common_test.vcxproj.filters‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,11 @@
3636
<ClCompile Include="..\..\..\src\yvalve\gds.cpp">
3737
<Filter>source</Filter>
3838
</ClCompile>
39+
<ClCompile Include="..\..\..\src\common\classes\tests\ClumpletTest.cpp">
40+
<Filter>source</Filter>
41+
</ClCompile>
42+
<ClCompile Include="..\..\..\src\common\classes\tests\VectorTest.cpp">
43+
<Filter>source</Filter>
44+
</ClCompile>
3945
</ItemGroup>
4046
</Project>

‎src/common/classes/ClumpletReader.cpp‎

Lines changed: 58 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,7 @@ void ClumpletReader::dump() const
8686

8787
try {
8888
ClumpletDump d(kind, getBuffer(), getBufferLength());
89-
int t = (kind == SpbStart || kind == UnTagged || kind == WideUnTagged || kind == SpbResponse || kind == InfoResponse) ?
90-
-1 : d.getBufferTag();
89+
const int t = isTagged() ? d.getBufferTag() : -1;
9190
gds__log("Tag=%d Offset=%d Length=%d Eof=%d\n", t, getCurOffset(), getBufferLength(), isEof());
9291
for (d.rewind(); !(d.isEof()); d.moveNext())
9392
{
@@ -98,7 +97,7 @@ void ClumpletReader::dump() const
9897
catch (const fatal_exception& x)
9998
{
10099
gds__log("Fatal exception during clumplet dump: %s", x.what());
101-
FB_SIZE_T l = getBufferLength() - getCurOffset();
100+
const FB_SIZE_T l = getBufferLength() - getCurOffset();
102101
const UCHAR *p = getBuffer() + getCurOffset();
103102
gds__log("Plain dump starting with offset %d: %s", getCurOffset(),
104103
ClumpletDump::hexString(p, l).c_str());
@@ -205,7 +204,7 @@ void ClumpletReader::invalid_structure(const char* what, const int data) const
205204
fatal_exception::raiseFmt("Invalid clumplet buffer structure: %s (%d)", what, data);
206205
}
207206

208-
bool ClumpletReader::isTagged() const
207+
bool ClumpletReader::isTagged() const noexcept
209208
{
210209
switch (kind)
211210
{
@@ -214,13 +213,17 @@ bool ClumpletReader::isTagged() const
214213
case WideTagged:
215214
case SpbAttach:
216215
return true;
216+
default:
217+
return false;
217218
}
218-
219-
return false;
220219
}
221220

222221
UCHAR ClumpletReader::getBufferTag() const
223222
{
223+
if (!isTagged()) {
224+
usage_mistake("buffer is not tagged");
225+
return 0;
226+
}
224227
const UCHAR* const buffer_end = getBufferEnd();
225228
const UCHAR* buffer_start = getBuffer();
226229

@@ -235,16 +238,6 @@ UCHAR ClumpletReader::getBufferTag() const
235238
return 0;
236239
}
237240
return buffer_start[0];
238-
case SpbStart:
239-
case UnTagged:
240-
case WideUnTagged:
241-
case SpbSendItems:
242-
case SpbReceiveItems:
243-
case SpbResponse:
244-
case InfoResponse:
245-
case InfoItems:
246-
usage_mistake("buffer is not tagged");
247-
return 0;
248241
case SpbAttach:
249242
if (buffer_end - buffer_start == 0)
250243
{
@@ -286,6 +279,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
286279
case isc_tpb_lock_timeout:
287280
case isc_tpb_at_snapshot_number:
288281
return TraditionalDpb;
282+
default:
283+
break;
289284
}
290285
return SingleTpb;
291286
case SpbSendItems:
@@ -298,20 +293,14 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
298293
case isc_info_length:
299294
case isc_info_flag_end:
300295
return SingleTpb;
296+
default:
297+
break;
301298
}
302299
return StringSpb;
303300
case SpbReceiveItems:
304301
case InfoItems:
305302
return SingleTpb;
306303
case SpbStart:
307-
switch(tag)
308-
{
309-
case isc_spb_auth_block:
310-
case isc_spb_trusted_auth:
311-
case isc_spb_auth_plugin_name:
312-
case isc_spb_auth_plugin_list:
313-
return Wide;
314-
}
315304
switch (spbState)
316305
{
317306
case 0:
@@ -345,6 +334,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
345334
case isc_spb_res_access_mode:
346335
case isc_spb_res_replica_mode:
347336
return ByteSpb;
337+
default:
338+
break;
348339
}
349340
invalid_structure("unknown parameter for backup/restore", tag);
350341
break;
@@ -363,6 +354,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
363354
case isc_spb_rpr_rollback_trans_64:
364355
case isc_spb_rpr_recover_two_phase_64:
365356
return BigIntSpb;
357+
default:
358+
break;
366359
}
367360
invalid_structure("unknown parameter for repair", tag);
368361
break;
@@ -388,6 +381,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
388381
case isc_spb_sec_groupid:
389382
case isc_spb_sec_admin:
390383
return IntSpb;
384+
default:
385+
break;
391386
}
392387
invalid_structure("unknown parameter for security database operation", tag);
393388
break;
@@ -414,6 +409,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
414409
case isc_spb_prp_online_mode:
415410
case isc_spb_prp_replica_mode:
416411
return ByteSpb;
412+
default:
413+
break;
417414
}
418415
invalid_structure("unknown parameter for setting database properties", tag);
419416
break;
@@ -428,6 +425,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
428425
return StringSpb;
429426
case isc_spb_options:
430427
return IntSpb;
428+
default:
429+
break;
431430
}
432431
invalid_structure("unknown parameter for getting statistics", tag);
433432
break;
@@ -450,6 +449,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
450449
return IntSpb;
451450
case isc_spb_nbk_clean_history:
452451
return SingleTpb;
452+
default:
453+
break;
453454
}
454455
invalid_structure("unknown parameter for nbackup", tag);
455456
break;
@@ -460,6 +461,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
460461
return StringSpb;
461462
case isc_spb_options:
462463
return IntSpb;
464+
default:
465+
break;
463466
}
464467
invalid_structure("unknown parameter for nbackup", tag);
465468
break;
@@ -474,7 +477,10 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
474477
return StringSpb;
475478
case isc_spb_trc_id:
476479
return IntSpb;
480+
default:
481+
break;
477482
}
483+
invalid_structure("unknown parameter for trace", tag);
478484
break;
479485
case isc_action_svc_validate:
480486
switch (tag)
@@ -487,7 +493,12 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
487493
return StringSpb;
488494
case isc_spb_val_lock_timeout:
489495
return IntSpb;
496+
default:
497+
break;
490498
}
499+
invalid_structure("unknown parameter for validate", tag);
500+
break;
501+
default:
491502
break;
492503
}
493504
invalid_structure("wrong spb state", spbState);
@@ -533,6 +544,8 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
533544
case isc_spb_tra_state:
534545
case isc_spb_tra_advise:
535546
return ByteSpb;
547+
default:
548+
break;
536549
}
537550
invalid_structure("unrecognized service response tag", tag);
538551
break;
@@ -543,26 +556,24 @@ ClumpletReader::ClumpletType ClumpletReader::getClumpletType(UCHAR tag) const
543556
case isc_info_truncated:
544557
case isc_info_flag_end:
545558
return SingleTpb;
559+
default:
560+
break;
546561
}
547562
return StringSpb;
563+
default:
564+
break;
548565
}
549566
invalid_structure("unknown clumplet kind", kind);
550567
return SingleTpb;
551568
}
552569

553570
void ClumpletReader::adjustSpbState()
554571
{
555-
switch (kind)
572+
if (kind == SpbStart &&
573+
spbState == 0 && // Just started with service start block ...
574+
getClumpletSize(true, true, true) == 1) // and this is action_XXX clumplet
556575
{
557-
case SpbStart:
558-
if (spbState == 0 && // Just started with service start block ...
559-
getClumpletSize(true, true, true) == 1) // and this is action_XXX clumplet
560-
{
561-
spbState = getClumpTag();
562-
}
563-
break;
564-
default:
565-
break;
576+
spbState = getClumpTag();
566577
}
567578
}
568579

@@ -582,7 +593,7 @@ FB_SIZE_T ClumpletReader::getClumpletSize(bool wTag, bool wLength, bool wData) c
582593
FB_SIZE_T lengthSize = 0;
583594
FB_SIZE_T dataSize = 0;
584595

585-
ClumpletType t = getClumpletType(clumplet[0]);
596+
const ClumpletType t = getClumpletType(clumplet[0]);
586597
switch (t)
587598
{
588599

@@ -651,13 +662,14 @@ FB_SIZE_T ClumpletReader::getClumpletSize(bool wTag, bool wLength, bool wData) c
651662

652663
default:
653664
invalid_structure("unknown clumplet type", t);
665+
return rc;
654666
}
655667

656668
const FB_SIZE_T total = 1 + lengthSize + dataSize;
657669
if (clumplet + total > buffer_end)
658670
{
659671
invalid_structure("buffer end before end of clumplet - clumplet too long", total);
660-
FB_SIZE_T delta = total - (buffer_end - clumplet);
672+
const FB_SIZE_T delta = total - (buffer_end - clumplet);
661673
if (delta > dataSize)
662674
dataSize = 0;
663675
else
@@ -678,50 +690,33 @@ void ClumpletReader::moveNext()
678690
if (isEof())
679691
return; // no need to raise useless exceptions
680692

681-
switch (kind)
693+
if (kind == InfoResponse)
682694
{
683-
case InfoResponse:
684695
switch (getClumpTag())
685696
{
686697
case isc_info_end:
687698
case isc_info_truncated:
688699
// terminating clumplet
689700
cur_offset = getBufferLength();
690701
return;
702+
default:
703+
break;
691704
}
692705
}
693706

694-
FB_SIZE_T cs = getClumpletSize(true, true, true);
707+
const FB_SIZE_T cs = getClumpletSize(true, true, true);
695708
adjustSpbState();
696709
cur_offset += cs;
697710
}
698711

699712
void ClumpletReader::rewind()
700713
{
701-
if (! getBuffer())
702-
{
714+
if (!getBuffer() || !isTagged())
703715
cur_offset = 0;
704-
spbState = 0;
705-
return;
706-
}
707-
switch (kind)
708-
{
709-
case UnTagged:
710-
case WideUnTagged:
711-
case SpbStart:
712-
case SpbSendItems:
713-
case SpbReceiveItems:
714-
case SpbResponse:
715-
case InfoResponse:
716-
case InfoItems:
717-
cur_offset = 0;
718-
break;
719-
default:
720-
if (kind == SpbAttach && getBufferLength() > 0 && getBuffer()[0] != isc_spb_version1)
721-
cur_offset = 2;
722-
else
723-
cur_offset = 1;
724-
}
716+
else if (kind == SpbAttach && getBufferLength() > 0 && getBuffer()[0] == isc_spb_version)
717+
cur_offset = 2;
718+
else
719+
cur_offset = 1;
725720
spbState = 0;
726721
}
727722

@@ -956,7 +951,7 @@ AuthReader::AuthReader(MemoryPool& pool, const AuthBlock& authBlock)
956951
rewind();
957952
}
958953

959-
static inline void erase(NoCaseString& s)
954+
static inline void erase(NoCaseString& s) noexcept
960955
{
961956
s.erase();
962957
}

0 commit comments

Comments
 (0)