Requirements review¶

Date of the review: 30/07/2015

Person who did the review: André Neto

Version of requirements: 0.3

Result of review: N/A

List of non-conformities: N/A

Comments: The requirements for this sprint are tracked in the EA model and are fully based on features that were already available in the previous version of the framework.

Architecture & design review¶

Date of the review: 30/07/2015

Person who did the review: André Neto

Version of architecture & design document: 0.3

Result of review: N/A

List of non-conformities: N/A

Comments: As per the original MARTe implementation, the user stories of this sprint are support functions and do not require formal design.

Note: No design document was produced but this was thoroughly discussed in several meetings and reported in #191. Discuss if this should be captured in EA, as a source of documentation (see #192).

Code and documentation review¶

Date of the review: 23/08/2015

Person who did the review: Ivan Herrero

Version of source code: 0.3

Result of review: PASS

List of non-conformities: N/A

• MARTe2-dev/Source/Core/L0Portability/ThreadsDatabase.cpp:92:85: Note 9025: More than two pointer indirection levels used for type 'ThreadInformation ***' [MISRA C++ Rule 5-0-19]
• MARTe2-dev/Source/Core/L0Portability/ThreadsDatabase.cpp:92:85: Note 929: cast from pointer to pointer [MISRA C++ Rule 5-2-7]
• MARTe2-dev/Source/Core/L0Portability/ThreadsDatabase.cpp:195:78: Note 925: cast from pointer to pointer [MISRA C++ Rule 5-2-8], [MISRA C++ Rule 5-2-9]
• MARTe2-dev/Source/Core/L0Portability/ThreadsDatabase.cpp:207:107: Note 9025: More than two pointer indirection levels used for type 'ThreadInformation ***' [MISRA C++ Rule 5-0-19]
• MARTe2-dev/Source/Core/L0Portability/ThreadsDatabase.cpp:207:107: Note 929: cast from pointer to pointer [MISRA C++ Rule 5-2-7]
• MARTe2-dev/Source/Core/L0Portability/ThreadsDatabase.cpp:207:118: Note 925: cast from pointer to pointer [MISRA C++ Rule 5-2-8], [MISRA C++ Rule 5-2-9]
• MARTe2-dev/Source/Core/L0Portability/ThreadInformation.h:47:25: Note 9109: type 'ThreadInformation' previously declared at location 'line 47' [MISRA C++ Rule 3-2-3]
• MARTe2-dev/Source/Core/L0Portability/ThreadInformation.cpp:93:49: Note 929: cast from pointer to pointer [MISRA C++ Rule 5-2-7]
• MARTe2-dev/Source/Core/L0Portability/ThreadInformation.h:47:25: Note 9109: type 'ThreadInformation' previously declared at location 'line 47' [MISRA C++ Rule 3-2-3]
• MARTe2-dev/Source/Core/L0Portability/ThreadInformation.h:47:25: Note 9109: type 'ThreadInformation' previously declared at location 'line 47' [MISRA C++ Rule 3-2-3]
• Warning 459: Function 'SystemThreadFunction(ThreadInformation *)' whose address was taken has an unprotected access to variable 'ThreadsDatabase::maxNOfEntries'
• Warning 459: Function 'SystemThreadFunction(ThreadInformation *)' whose address was taken has an unprotected access to variable 'ThreadsDatabase::nOfEntries'
• Warning 459: Function 'SystemThreadFunction(ThreadInformation *)' whose address was taken has an unprotected access to variable 'ThreadsDatabase::entries'
• Warning 459: Function 'SystemThreadFunction(ThreadInformation *)' whose address was taken has an unprotected access to variable 'ThreadsDatabase::internalMutex'
• Threads::name declares that it returns a C style string (char8*) without specifying who is the responsible for the management of the memory used by this char array (it actually returns a pointer to the char array hosted by an instance of ThreadInformation calling its ThreadName method). Perhaps it should be created a copy of the char array by means of Memory::StringDup or changed the prototype putting the char array as an out parameter of the method.
• Warning 1502: defined object 'ObjectHeap_' of type 'Heap' has no nonstatic data members
• Warning 1502: defined object 'ReferenceContainerHeap_' of type 'Heap' has no nonstatic data members
• Note 974: Worst case function for stack usage: 'ReferenceContainer::Find' is recursive, loop contains call to 'ReferenceContainer::Find'. See stack for a full report. [MISRA C+ Rule 7-5-4]
• The global FastPollingMutexSem in Object.cpp could be a class attribute.
• In the function DecrementReferences it would be better to initialise ret to referencesNumber.
• The implementation of the Heap was the minimum required to be able to compile, but this will be reviewed in v0.4

Unit test review¶

Date of the review: 23/08/2015

Person who did the review: Andre' Neto

Version of unit tests: 0.3

Result of coverage tests review: PASS

Result of functional tests review: PASS

Result of review: PASS

List of non-conformities:

• ThreadsDatabase class has not specific unit tests, but it is actually tested through the unit tests of Threads class.
• ThreadInformation class has not specific unit tests, but it is actually tested through the unit tests of Threads class.
• Unit testing coverage note: Error paths not exercised on:
  • ThreadsOS.cpp (missing test of priorities and ProcessorType::GetDefaultCPUs())
  • StringPortable.cpp (missing test for Concatenation with a NULL string)
• Unit testing coverage note: errors which require forcing an operating system fault are not tested: SleepOS.cpp, ThreadsOS.cpp, MutexSemOS.cpp, LoadableLibraryOS.h, EventSemOS.cpp.
• Unit testing coverage note: The following can only be tested when DLL support, configuration and introspection are implemented: ClassRegistryDatabase.cpp, ReferenceT.h, Object.cpp.
• Unit testing coverage note: The following are not tested because they have a private interface to disallow usage by the end-user: ClassRegistryItem.cpp, Object.cpp, ReferenceT.h. These have been removed from lcov using the markers //LCOV_EXCL_START and //LCOV_EXCL_STOP
• Unit testing coverage note: The delete object destructors (mangled with *D0Ev, see http://stackoverflow.com/questions/6613870/gnu-gcc-g-why-does-it-generate-multiple-dtors) are not being tested. This results in a low coverage function for many classes: Iterator.h, ReferenceContainerFilterReferences.h, SearchFilter.h and SortFilter.h
• Unit testing coverage note: In Processor::Family() the sentences inside the block protected by "if (family == 0xf)" has not been exercised, because the processor's family of the processor used in tests has not reached 0xf, so it does not need to use the Extended Family ID.
• Threads unit tests fails at:
  [ FAILED ] ThreadsGTest.TestPriority
  [ FAILED ] ThreadsGTest.TestGetThreadInfoCopy
  The reason why these tests fail, is that in Linux a regular user is not allowed to change the priority. This can be solved by either running the tests as the root user (not advisable), or by editing the file /etc/security/limits.conf and adding the following lines (change aneto to your username):
  

  @aneto soft rtprio 100
  @aneto hard rtprio 10
  

  
  [ FAILED ] BasicConsoleGTest.TestOpenModePerformCharacterInput (only happens in the continuous integration server)
• Testing note: No proper test can be done for the LoadableLibrary::Close() function. It will have to be discussed in the future.
• The following methods of BasicConsole do not have an explicit test, because the answer is different according to the target operating system:
  

  bool ColourSupported()
  bool ConsoleBufferSupported()
  bool CursorPositionSupported()
  bool TitleBarSupported()
  bool WindowSizeSupported()
  bool TimeoutSupported()
  

• The following methods of BasicConsole are not tested because they are not implemented in Linux (the setting of colour could be implemented in Linux, see FreeRTOS implementation):
  

  ErrorType BasicConsole::ShowBuffer();
  ErrorType BasicConsole::SetColour(const Colours &foregroundColour, const Colours &backgroundColour);
  ErrorType BasicConsole::SetTitleBar(const char8 * const title);
  ErrorType BasicConsole::GetTitleBar(char8 * const title, const uint32 &size) const;
  ErrorType BasicConsole::SetCursorPosition(const uint32 &column, const uint32 &row);
  ErrorType BasicConsole::GetCursorPosition(uint32 &column, uint32 &row) const;
  ErrorType BasicConsole::SetWindowSize(const uint32 &numberOfColumns, const uint32 &numberOfRows);
  ErrorType BasicConsole::GetWindowSize(uint32 &numberOfColumns, uint32 &numberOfRows) const;
  ErrorType BasicConsole::PlotChar(const char8 &c, const Colours &foregroundColour, const Colours &backgroundColour, const uint32 &column, const uint32 &row);
  

• Notes: The class HighResolutionTimerCalibratorOS does not have a counterpart on L0, so it is not directly tested because the target of the unit tests are only the classes on L0.
• Note: The tests for HighResolutionTimer::TestCounter, HighResolutionTimer::TestCounter32, and HighResolutionTimer::TestGetTimeStamp, sometimes pass and sometimes fail. The reason is that the operating system dynamically changes the CPU frequency (for power-saving reasons). As a consequence, the frequency retrieved by MARTe when the test starts might not be the same as when the test is executed, generating false errors. This can be resolved by issuing the following command in CentOS: cpupower frequency-set -g performance

Integration test review¶

Date of the review: 23/08/2015

Person who did the review: Andre' Neto

Version of integration tests: 0.3

Result of review: N/A

List of non-conformities: N/A

Comments: As per the original MARTe implementation, the user stories of this sprint are support functions and do not require formal integration tests.

Acceptance test review¶

Date of the review: 23/08/2015

Person who did the review: Andre' Neto

Version of acceptance tests: 0.3

Result of review: N/A

List of non-conformities: N/A

Comments: As per the original MARTe implementation, the user stories of this sprint are support functions and do not require formal acceptance tests.