Skip to content

Commit

Permalink
Merge pull request nasa#228 from skliper/fix216-strict_cppcheck
Browse files Browse the repository at this point in the history 
Fix nasa#216, nasa#227 - Adds strict cppcheck and fixes remaining warnings (replace codec macros)

- HOTFIX nasa#228-1, remove `/` prefix to fsw from strict-dir-list input
- HOTFIX nasa#228-2, fix cppcheck warning for declaration-definition name
mismatch of `CF_CFDP_DecodeCrc` argument-2
  • Loading branch information
@astrogeco
astrogeco committed Apr 18, 2022
2 parents 869e9d4 + d5439a9 commit 9233e7c
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 47 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/static-analysis.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ jobs:
static-analysis:
name: Run cppcheck
uses: nasa/cFS/.github/workflows/static-analysis.yml@main

with:
strict-dir-list: 'fsw'
80 changes: 36 additions & 44 deletions fsw/src/cf_codec.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ static inline void CF_Codec_Store_uint8(CF_CFDP_uint8_t *pdst, uint8 val)
{
pdst->octets[0] = val;
}
#define cfdp_set_uint8(dst, src) (CF_Codec_Store_uint8(&(dst), src))

/*----------------------------------------------------------------
*
Expand All @@ -149,7 +148,6 @@ static inline void CF_Codec_Store_uint16(CF_CFDP_uint16_t *pdst, uint16 val)
val >>= 8;
pdst->octets[0] = val & 0xFF;
}
#define cfdp_set_uint16(dst, src) (CF_Codec_Store_uint16(&(dst), src))

/*----------------------------------------------------------------
*
Expand All @@ -168,7 +166,6 @@ static inline void CF_Codec_Store_uint32(CF_CFDP_uint32_t *pdst, uint32 val)
val >>= 8;
pdst->octets[0] = val & 0xFF;
}
#define cfdp_set_uint32(dst, src) (CF_Codec_Store_uint32(&(dst), src))

/*----------------------------------------------------------------
*
Expand All @@ -195,7 +192,6 @@ static inline void CF_Codec_Store_uint64(CF_CFDP_uint64_t *pdst, uint64 val)
val >>= 8;
pdst->octets[0] = val & 0xFF;
}
#define cfdp_set_uint64(dst, src) (CF_Codec_Store_uint64(&(dst), src))

/*----------------------------------------------------------------
*
Expand All @@ -208,7 +204,6 @@ static inline void CF_Codec_Load_uint8(uint8 *pdst, const CF_CFDP_uint8_t *psrc)
{
*pdst = psrc->octets[0];
}
#define cfdp_get_uint8(dst, src) (CF_Codec_Load_uint8(&(dst), &(src)))

/*----------------------------------------------------------------
*
Expand All @@ -227,7 +222,6 @@ static inline void CF_Codec_Load_uint16(uint16 *pdst, const CF_CFDP_uint16_t *ps

*pdst = val;
}
#define cfdp_get_uint16(dst, src) (CF_Codec_Load_uint16(&(dst), &(src)))

/*----------------------------------------------------------------
*
Expand All @@ -250,7 +244,6 @@ static inline void CF_Codec_Load_uint32(uint32 *pdst, const CF_CFDP_uint32_t *ps

*pdst = val;
}
#define cfdp_get_uint32(dst, src) (CF_Codec_Load_uint32(&(dst), &(src)))

/*----------------------------------------------------------------
*
Expand Down Expand Up @@ -281,7 +274,6 @@ static inline void CF_Codec_Load_uint64(uint64 *pdst, const CF_CFDP_uint64_t *ps

*pdst = val;
}
#define cfdp_get_uint64(dst, src) (CF_Codec_Load_uint64(&(dst), &(src)))

/*----------------------------------------------------------------
*
Expand Down Expand Up @@ -410,14 +402,14 @@ void CF_CFDP_EncodeHeaderWithoutSize(CF_EncoderState_t *state, CF_Logical_PduHea
peh = CF_ENCODE_FIXED_CHUNK(state, CF_CFDP_PduHeader_t);
if (peh != NULL)
{
cfdp_set_uint8(peh->flags, 0);
CF_Codec_Store_uint8(&(peh->flags), 0);
FSV(peh->flags, CF_CFDP_PduHeader_FLAGS_VERSION, plh->version);
FSV(peh->flags, CF_CFDP_PduHeader_FLAGS_DIR, plh->direction);
FSV(peh->flags, CF_CFDP_PduHeader_FLAGS_TYPE, plh->pdu_type);
FSV(peh->flags, CF_CFDP_PduHeader_FLAGS_MODE, plh->txm_mode);

/* The eid+tsn lengths are encoded as -1 */
cfdp_set_uint8(peh->eid_tsn_lengths, 0);
CF_Codec_Store_uint8(&(peh->eid_tsn_lengths), 0);
FSV(peh->eid_tsn_lengths, CF_CFDP_PduHeader_LENGTHS_ENTITY, plh->eid_length - 1);
FSV(peh->eid_tsn_lengths, CF_CFDP_PduHeader_LENGTHS_TRANSACTION_SEQUENCE, plh->txn_seq_length - 1);

Expand Down Expand Up @@ -457,7 +449,7 @@ void CF_CFDP_EncodeHeaderFinalSize(CF_EncoderState_t *state, CF_Logical_PduHeade
peh = (CF_CFDP_PduHeader_t *)state->base;

/* Total length is a simple 16-bit quantity */
cfdp_set_uint16(peh->length, plh->data_encoded_length);
CF_Codec_Store_uint16(&(peh->length), plh->data_encoded_length);
}

/* This "closes" the packet so nothing else can be added to this EncoderState,
Expand All @@ -481,7 +473,7 @@ void CF_CFDP_EncodeFileDirectiveHeader(CF_EncoderState_t *state, CF_Logical_PduF
peh = CF_ENCODE_FIXED_CHUNK(state, CF_CFDP_PduFileDirectiveHeader_t);
if (peh != NULL)
{
cfdp_set_uint8(peh->directive_code, value);
CF_Codec_Store_uint8(&(peh->directive_code), value);
}
}

Expand All @@ -501,7 +493,7 @@ void CF_CFDP_EncodeLV(CF_EncoderState_t *state, CF_Logical_Lv_t *pllv)
lv = CF_ENCODE_FIXED_CHUNK(state, CF_CFDP_lv_t);
if (lv != NULL)
{
cfdp_set_uint8(lv->length, pllv->length);
CF_Codec_Store_uint8(&(lv->length), pllv->length);
if (pllv->length > 0)
{
data_ptr = CF_CFDP_DoEncodeChunk(state, pllv->length);
Expand Down Expand Up @@ -533,8 +525,8 @@ void CF_CFDP_EncodeTLV(CF_EncoderState_t *state, CF_Logical_Tlv_t *pltlv)
tlv = CF_ENCODE_FIXED_CHUNK(state, CF_CFDP_tlv_t);
if (tlv != NULL)
{
cfdp_set_uint8(tlv->type, pltlv->type);
cfdp_set_uint8(tlv->length, pltlv->length);
CF_Codec_Store_uint8(&(tlv->type), pltlv->type);
CF_Codec_Store_uint8(&(tlv->length), pltlv->length);

/* the only TLV type currently implemented is entity id */
if (pltlv->type == CF_CFDP_TLV_TYPE_ENTITY_ID)
Expand Down Expand Up @@ -572,8 +564,8 @@ void CF_CFDP_EncodeSegmentRequest(CF_EncoderState_t *state, CF_Logical_SegmentRe
sr = CF_ENCODE_FIXED_CHUNK(state, CF_CFDP_SegmentRequest_t);
if (sr != NULL)
{
cfdp_set_uint32(sr->offset_start, plseg->offset_start);
cfdp_set_uint32(sr->offset_end, plseg->offset_end);
CF_Codec_Store_uint32(&(sr->offset_start), plseg->offset_start);
CF_Codec_Store_uint32(&(sr->offset_end), plseg->offset_end);
}
}

Expand Down Expand Up @@ -628,10 +620,10 @@ void CF_CFDP_EncodeMd(CF_EncoderState_t *state, CF_Logical_PduMd_t *plmd)
md = CF_ENCODE_FIXED_CHUNK(state, CF_CFDP_PduMd_t);
if (md != NULL)
{
cfdp_set_uint8(md->segmentation_control, 0);
CF_Codec_Store_uint8(&(md->segmentation_control), 0);
FSV(md->segmentation_control, CF_CFDP_PduMd_CLOSURE_REQUESTED, plmd->close_req);
FSV(md->segmentation_control, CF_CFDP_PduMd_CHECKSUM_TYPE, plmd->checksum_type);
cfdp_set_uint32(md->size, plmd->size);
CF_Codec_Store_uint32(&(md->size), plmd->size);

/* Add in LV for src/dest */
CF_CFDP_EncodeLV(state, &plmd->source_filename);
Expand Down Expand Up @@ -664,7 +656,7 @@ void CF_CFDP_EncodeFileDataHeader(CF_EncoderState_t *state, bool with_meta, CF_L

if (optional_fields != NULL)
{
cfdp_set_uint8(*optional_fields, 0);
CF_Codec_Store_uint8(optional_fields, 0);
FSV(*optional_fields, CF_CFDP_PduFileData_RECORD_CONTINUATION_STATE, plfd->continuation_state);
FSV(*optional_fields, CF_CFDP_PduFileData_SEGMENT_METADATA_LENGTH, plfd->segment_list.num_segments);

Expand All @@ -674,7 +666,7 @@ void CF_CFDP_EncodeFileDataHeader(CF_EncoderState_t *state, bool with_meta, CF_L
fd = CF_ENCODE_FIXED_CHUNK(state, CF_CFDP_PduFileDataHeader_t);
if (fd != NULL)
{
cfdp_set_uint32(fd->offset, plfd->offset);
CF_Codec_Store_uint32(&(fd->offset), plfd->offset);
}
}

Expand All @@ -693,10 +685,10 @@ void CF_CFDP_EncodeEof(CF_EncoderState_t *state, CF_Logical_PduEof_t *pleof)
eof = CF_ENCODE_FIXED_CHUNK(state, CF_CFDP_PduEof_t);
if (eof != NULL)
{
cfdp_set_uint8(eof->cc, 0);
CF_Codec_Store_uint8(&(eof->cc), 0);
FSV(eof->cc, CF_CFDP_PduEof_FLAGS_CC, pleof->cc);
cfdp_set_uint32(eof->crc, pleof->crc);
cfdp_set_uint32(eof->size, pleof->size);
CF_Codec_Store_uint32(&(eof->crc), pleof->crc);
CF_Codec_Store_uint32(&(eof->size), pleof->size);

CF_CFDP_EncodeAllTlv(state, &pleof->tlv_list);
}
Expand All @@ -717,7 +709,7 @@ void CF_CFDP_EncodeFin(CF_EncoderState_t *state, CF_Logical_PduFin_t *plfin)
fin = CF_ENCODE_FIXED_CHUNK(state, CF_CFDP_PduFin_t);
if (fin != NULL)
{
cfdp_set_uint8(fin->flags, 0);
CF_Codec_Store_uint8(&(fin->flags), 0);
FSV(fin->flags, CF_CFDP_PduFin_FLAGS_CC, plfin->cc);
FSV(fin->flags, CF_CFDP_PduFin_FLAGS_DELIVERY_CODE, plfin->delivery_code);
FSV(fin->flags, CF_CFDP_PduFin_FLAGS_FILE_STATUS, plfin->file_status);
Expand All @@ -741,11 +733,11 @@ void CF_CFDP_EncodeAck(CF_EncoderState_t *state, CF_Logical_PduAck_t *plack)
ack = CF_ENCODE_FIXED_CHUNK(state, CF_CFDP_PduAck_t);
if (ack != NULL)
{
cfdp_set_uint8(ack->directive_and_subtype_code, 0);
CF_Codec_Store_uint8(&(ack->directive_and_subtype_code), 0);
FSV(ack->directive_and_subtype_code, CF_CFDP_PduAck_DIR_CODE, plack->ack_directive_code);
FSV(ack->directive_and_subtype_code, CF_CFDP_PduAck_DIR_SUBTYPE_CODE, plack->ack_subtype_code);

cfdp_set_uint8(ack->cc_and_transaction_status, 0);
CF_Codec_Store_uint8(&(ack->cc_and_transaction_status), 0);
FSV(ack->cc_and_transaction_status, CF_CFDP_PduAck_CC, plack->cc);
FSV(ack->cc_and_transaction_status, CF_CFDP_PduAck_TRANSACTION_STATUS, plack->txn_status);
}
Expand All @@ -766,8 +758,8 @@ void CF_CFDP_EncodeNak(CF_EncoderState_t *state, CF_Logical_PduNak_t *plnak)
nak = CF_ENCODE_FIXED_CHUNK(state, CF_CFDP_PduNak_t);
if (nak != NULL)
{
cfdp_set_uint32(nak->scope_start, plnak->scope_start);
cfdp_set_uint32(nak->scope_end, plnak->scope_end);
CF_Codec_Store_uint32(&(nak->scope_start), plnak->scope_start);
CF_Codec_Store_uint32(&(nak->scope_end), plnak->scope_end);

CF_CFDP_EncodeAllSegments(state, &plnak->segment_list);
}
Expand All @@ -788,7 +780,7 @@ void CF_CFDP_EncodeCrc(CF_EncoderState_t *state, uint32 *plcrc)
pecrc = CF_ENCODE_FIXED_CHUNK(state, CF_CFDP_uint32_t);
if (pecrc != NULL)
{
cfdp_set_uint32(*pecrc, *plcrc);
CF_Codec_Store_uint32(pecrc, *plcrc);
}
}

Expand Down Expand Up @@ -850,7 +842,7 @@ void CF_CFDP_DecodeHeader(CF_DecoderState_t *state, CF_Logical_PduHeader_t *plh)
plh->txn_seq_length = FGV(peh->eid_tsn_lengths, CF_CFDP_PduHeader_LENGTHS_TRANSACTION_SEQUENCE) + 1;

/* Length is a simple 16-bit quantity and refers to the content after this header */
cfdp_get_uint16(plh->data_encoded_length, peh->length);
CF_Codec_Load_uint16(&(plh->data_encoded_length), &(peh->length));

/* Now copy variable-length fields */
plh->source_eid = CF_DecodeIntegerInSize(state, plh->eid_length);
Expand Down Expand Up @@ -879,7 +871,7 @@ void CF_CFDP_DecodeFileDirectiveHeader(CF_DecoderState_t *state, CF_Logical_PduF
peh = CF_DECODE_FIXED_CHUNK(state, CF_CFDP_PduFileDirectiveHeader_t);
if (peh != NULL)
{
cfdp_get_uint8(packet_val, peh->directive_code);
CF_Codec_Load_uint8(&packet_val, &(peh->directive_code));
pfdir->directive_code = packet_val;
}
}
Expand All @@ -899,7 +891,7 @@ void CF_CFDP_DecodeLV(CF_DecoderState_t *state, CF_Logical_Lv_t *pllv)
lv = CF_DECODE_FIXED_CHUNK(state, CF_CFDP_lv_t);
if (lv != NULL)
{
cfdp_get_uint8(pllv->length, lv->length);
CF_Codec_Load_uint8(&(pllv->length), &(lv->length));
pllv->data_ptr = CF_CFDP_DoDecodeChunk(state, pllv->length);
}
}
Expand All @@ -920,8 +912,8 @@ void CF_CFDP_DecodeTLV(CF_DecoderState_t *state, CF_Logical_Tlv_t *pltlv)
tlv = CF_DECODE_FIXED_CHUNK(state, CF_CFDP_tlv_t);
if (tlv != NULL)
{
cfdp_get_uint8(type_val, tlv->type);
cfdp_get_uint8(pltlv->length, tlv->length);
CF_Codec_Load_uint8(&type_val, &(tlv->type));
CF_Codec_Load_uint8(&(pltlv->length), &(tlv->length));

/* the only TLV type currently implemented is entity id */
pltlv->type = type_val;
Expand Down Expand Up @@ -953,8 +945,8 @@ void CF_CFDP_DecodeSegmentRequest(CF_DecoderState_t *state, CF_Logical_SegmentRe
sr = CF_DECODE_FIXED_CHUNK(state, CF_CFDP_SegmentRequest_t);
if (sr != NULL)
{
cfdp_get_uint32(plseg->offset_start, sr->offset_start);
cfdp_get_uint32(plseg->offset_end, sr->offset_end);
CF_Codec_Load_uint32(&(plseg->offset_start), &(sr->offset_start));
CF_Codec_Load_uint32(&(plseg->offset_end), &(sr->offset_end));
}
}

Expand All @@ -975,7 +967,7 @@ void CF_CFDP_DecodeMd(CF_DecoderState_t *state, CF_Logical_PduMd_t *plmd)
{
plmd->close_req = FGV(md->segmentation_control, CF_CFDP_PduMd_CLOSURE_REQUESTED);
plmd->checksum_type = FGV(md->segmentation_control, CF_CFDP_PduMd_CHECKSUM_TYPE);
cfdp_get_uint32(plmd->size, md->size);
CF_Codec_Load_uint32(&(plmd->size), &(md->size));

/* Add in LV for src/dest */
CF_CFDP_DecodeLV(state, &plmd->source_filename);
Expand Down Expand Up @@ -1040,7 +1032,7 @@ void CF_CFDP_DecodeFileDataHeader(CF_DecoderState_t *state, bool with_meta, CF_L
fd = CF_DECODE_FIXED_CHUNK(state, CF_CFDP_PduFileDataHeader_t);
if (fd != NULL)
{
cfdp_get_uint32(plfd->offset, fd->offset);
CF_Codec_Load_uint32(&(plfd->offset), &(fd->offset));

plfd->data_len = CF_CODEC_GET_REMAIN(state);
plfd->data_ptr = CF_CFDP_DoDecodeChunk(state, plfd->data_len);
Expand All @@ -1062,7 +1054,7 @@ void CF_CFDP_DecodeCrc(CF_DecoderState_t *state, uint32 *plcrc)
pecrc = CF_DECODE_FIXED_CHUNK(state, CF_CFDP_uint32_t);
if (pecrc != NULL)
{
cfdp_get_uint32(*plcrc, *pecrc);
CF_Codec_Load_uint32(plcrc, pecrc);
}
}

Expand All @@ -1082,8 +1074,8 @@ void CF_CFDP_DecodeEof(CF_DecoderState_t *state, CF_Logical_PduEof_t *pleof)
if (eof != NULL)
{
pleof->cc = FGV(eof->cc, CF_CFDP_PduEof_FLAGS_CC);
cfdp_get_uint32(pleof->crc, eof->crc);
cfdp_get_uint32(pleof->size, eof->size);
CF_Codec_Load_uint32(&(pleof->crc), &(eof->crc));
CF_Codec_Load_uint32(&(pleof->size), &(eof->size));

CF_CFDP_DecodeAllTlv(state, &pleof->tlv_list, CF_PDU_MAX_TLV);
}
Expand Down Expand Up @@ -1150,8 +1142,8 @@ void CF_CFDP_DecodeNak(CF_DecoderState_t *state, CF_Logical_PduNak_t *plnak)
nak = CF_DECODE_FIXED_CHUNK(state, CF_CFDP_PduNak_t);
if (nak != NULL)
{
cfdp_get_uint32(plnak->scope_start, nak->scope_start);
cfdp_get_uint32(plnak->scope_end, nak->scope_end);
CF_Codec_Load_uint32(&(plnak->scope_start), &(nak->scope_start));
CF_Codec_Load_uint32(&(plnak->scope_end), &(nak->scope_end));

CF_CFDP_DecodeAllSegments(state, &plnak->segment_list, CF_PDU_MAX_SEGMENTS);
}
Expand Down
4 changes: 2 additions & 2 deletions fsw/src/cf_codec.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -816,8 +816,8 @@ void CF_CFDP_DecodeNak(CF_DecoderState_t *state, CF_Logical_PduNak_t *plnak);
* decoder is not changed.
*
* @param state Decoder state object
* @param pcrc Pointer to logical CRC value
* @param plcrc Pointer to logical CRC value
*/
void CF_CFDP_DecodeCrc(CF_DecoderState_t *state, uint32 *pcrc);
void CF_CFDP_DecodeCrc(CF_DecoderState_t *state, uint32 *plcrc);

#endif /* !CF_CODEC_H */

0 comments on commit 9233e7c

Please sign in to comment.