Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recommended lgtm issues #15

Closed
avan989 opened this issue Dec 20, 2019 · 1 comment · Fixed by #26 or #29
Closed

Recommended lgtm issues #15

avan989 opened this issue Dec 20, 2019 · 1 comment · Fixed by #26 or #29
Labels
bug Something isn't working
Milestone

Comments

@avan989
Copy link

avan989 commented Dec 20, 2019

Is your feature request related to a problem? Please describe.
Recommended issue from lgtm:
cfe_ts_crc.c:

switch(TypeCRC)
--
114 | {
115 | /*       case CFE_ES_CRC_32:                                                    */
116 | /*            CFE_ES_WriteToSysLog("CFE ES Calculate CRC32 not Implemented\n"); */
117 | /*            break;                                                            */
118 |  
119 | case CFE_ES_CRC_16:
120 | Crc    =  (int16 )( 0xFFFF & InputCRC );
121 | BufPtr = (uint8 *)DataPtr;
122 |  
123 | for ( i = 0 ; i < DataLength ; i++,  BufPtr++)
124 | {
125 | Index = ( ( Crc ^ *BufPtr) & 0x00FF);
126 | Crc = ( (Crc >> 8 ) & 0x00FF) ^ CrcTable[Index];
127 | }
128 | break;
129 |  
130 | /*       case CFE_ES_CRC_8:                                                    */
131 | /*            CFE_ES_WriteToSysLog("CFE ES Calculate CRC8 not Implemented\n"); */
132 | /*            break;                                                           */
133 |  
134 | default:
135 | break;
136 | }
  | This switch statement should either handle more cases, or be rewritten as an if statement.


{
--
115 | /*       case CFE_ES_CRC_32:                                                    */
116 | /*            CFE_ES_WriteToSysLog("CFE ES Calculate CRC32 not Implemented\n"); */
  | This comment appears to contain commented-out code
117 | /*            break;                                                            */
  | This comment appears to contain commented-out code



/*       case CFE_ES_CRC_8:                                                    */
--
131 | /*            CFE_ES_WriteToSysLog("CFE ES Calculate CRC8 not Implemented\n"); */
  | This comment appears to contain commented-out code
132 | /*            break;                                                           */
  | This comment appears to contain commented-out code



Requester Info
Anh Van, NASA Goddard

@skliper
Copy link
Contributor

skliper commented Dec 22, 2019

Code is messy to begin with, suggest just simplifying to only do CRC_16. If other options are added in the future update the code at that time (keep simple for now).

skliper added a commit to skliper/tblCRCTool that referenced this issue Jan 6, 2021
@astrogeco astrogeco added the bug Something isn't working label Jan 21, 2021
@astrogeco astrogeco added this to the 1.3.0 milestone Jan 21, 2021
astrogeco added a commit that referenced this issue Jan 21, 2021
Fix #15, Remove unimplemented crc cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants