-
Notifications
You must be signed in to change notification settings - Fork 289
gPTP code comments
David Cemin edited this page Jun 17, 2015
·
50 revisions
This page is a public site to politely record comments on the gPTP code base
| Module | Function | Submitter comment | Reviewer comment | Action |
|---|---|---|---|---|
| Module | Function | Submitter comment | Reviewer comment | Action |
|---|---|---|---|---|
| avbts_message.hpp | FollowUpTLV::toByteString() | memcpy(byte_str, this, sizeof(*this)) operation looks weird (AndrewE) | ||
| avbts_osnet.hpp | LinkLayerAddress::toOctetArray and LinkLayerAddress(uint8_t *) | Method does not verify if pointer is valid. Might segfault(DavidC) | It should be fixed | Issue #241 |
| avbts_osnet.hpp | InterfaceName::~InterfaceName() | destructor does not delete this->name created at the constructor (DavidC) | It should be fixed | Issue #242 |
| avbts_osnet.hpp | InterfaceName::toString | if string is null, strncpy might fail silently and the method returns successfully (DavidC) | It should be fixed | Issue #241 |
| PTPMessageCommon | PTPMessageCommon::buildCommonHeader | correctionField assumes Little-endian Machine. We should be able to detect endianness and set value accordingly (DavidC) | It should be fixed | Issue #243 |
| Description | Submitter comment | Reviewer comment | Action |
|---|---|---|---|
| Cleanup the message build/parse code – Could be much smaller and simpler using struct to represent message | by ChrisH | ||
| Inconsistent Port Identity and Clock Identity code – in some code clock identity is treated as a string in others as a C++ object | by ChrisH | ||
| Inconsistent timestamp mathematics – mixture of C++ operator overloading and macros | by ChrisH | ||
| Timer API design and usage could be simplified | by ChrisH | ||
| Change “low-level” APIs to conform to those in AVnu docs | by ChrisH | ||
| Change #defines in code to conform to IEEE standards doc | by ChrisH | ||
| Remove duplicate struct gPtpTimeData defns | by AndrewE | ||
| Magic numbers at ieee1588Port constructor | by DavidC | It should be fixed | Issue #129 |
| Method IEEE1588Port::recoverPort doesnt do anything. Just returns. | by DavidC | ||
| Method IEEE1588Port::addForeignMaster not implemented. | by DavidC | ||
| Method IEEE1588Port::removeForeignMaster not implemented. | by DavidC | ||
| Method IEEE1588Port::removeForeignMasterAll not implemented. | by DavidC | ||
| Method IEEE1588Port::getParentLastSyncSequenceNumber not implemented. | by DavidC | ||
| Method IEEE1588Port::setParentLastSyncSequenceNumber not implemented. | by DavidC | ||
| Magic number being added to messageType | by DavidC | It should be fixed | Issue #129 |
| Improve code clarity at buildPTPMessage | by DavidC | ||
| Improve code clarity at buildPTPMessage | by DavidC | ||
| Improve code clarity at buildPTPMessage | by DavidC | ||
| Magic number at buildPTPMessage | by DavidC | It should be fixed | Issue #129 |
| Remove commented code at buildPTPMessage if not in use | by DavidC | ||
| Duplicated define at OUTSTANDING_MESSAGES from OUTSTANDING_MESSAGES | by DavidC | ||
| ClockIdentity set() doesn't have matching get(). getIdentityString() should have matching setIDentityString() | by DavidC | ||
| Should msg pointer be const at IEEE1588Port::setLastSync(PTPMessageSync *msg) ? | by DavidC | ||
| HWTimestamper_final not used | by DavidC | ||
| Should we remove unused code as for instance scaledNs ? | by DavidC | ||
| HWTimestamper_get_extderror not in use. | by DavidC | ||
| Remove HWTimestamper_get_extclk_offset. It was a hack to get a specific board working. | by DavidC | ||
| Description | Submitter comment | Reviewer comment | Action |
|---|---|---|---|
| Encapsulate the formatting of data into byte buffers/byte streams into a small library of functions | Levi | ||
| [daemon_cl.cpp] Indent code between lines 273 and 323 (open-avb-next @ 56f990a) | DavidC | ||
| add comments describing difference between linux_hal_common.cpp and linux_hal_genric.cpp | AndrewE | ||
| ieee1588port.cpp has different indentation throughout the file. Some functions are using 2 spaces and some are using 1 tab of 4 spaces. | DavidC | ||
| Evaluate commented code at [IEEE1588Port::becomeSlave] (https://github.com/AVnu/Open-AVB/blob/open-avb-next/daemons/gptp/common/ieee1588port.cpp#L944) | DavidC | ||
| Evaluate commented code at PTPMessageCommon::buildCommonHeader | DavidC | ||
| Remove the double semi-colon at PTPMessageCommon::getTimestampCounterValue. Probably a typo. | DavidC |