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

196 reducing cyclomatic complexity #232

Merged
merged 30 commits into from
Mar 28, 2024
Merged

Conversation

rjbrown2
Copy link
Contributor

This is missing some DOxygen boilerplate, but I'll add that in after this has been reviewed.
If you guys could review function names, and the refactoring in general that would be helpful. There will be additional TM requests in the near future. This merge mainly deals with TC, and a couple of other minor refactors:

crypto_tc:

Crypto_TC_Apply_Security_Cam
Crypto_TC_Process_Security_Cam
sadb_routine_inmemory_template.c:

sadb_get_operational_sa_from_gvcid
crypto.c:

Crypto_Check_Anti_Replay
cryptography_interface_lib_gcrypt_template.c:

cryptography_aead_encrypt
cryptography_validate_authentication
cryptography_aead_decrypto.c
base64url.c:

base64urlDecode
standalone.c:

crypto_standalone_tm_process

@rjbrown2
Copy link
Contributor Author

Standby for fixes to the two failing build tests.

src/core/crypto.c Outdated Show resolved Hide resolved
src/core/crypto.c Outdated Show resolved Hide resolved
@jlucas9
Copy link
Collaborator

jlucas9 commented Mar 26, 2024

Do we have pre/post complexity numbers that we could capture here as well?

Copy link
Contributor

@dccutrig dccutrig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a few comments. Big changes to TC but we expected that

@rjbrown2
Copy link
Contributor Author

rjbrown2 commented Mar 26, 2024

Do we have pre/post complexity numbers that we could capture here as well?

I have the original XML and its numbers/findings within CAP. I added an additional column for tracking the changes.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 73.33333% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 72.93%. Comparing base (11a14ef) to head (bd84fd2).

Files Patch % Lines
src/core/crypto.c 73.33% 6 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #232      +/-   ##
==========================================
+ Coverage   72.00%   72.93%   +0.93%     
==========================================
  Files           8        8              
  Lines        2668     2864     +196     
  Branches      427      442      +15     
==========================================
+ Hits         1921     2089     +168     
- Misses        583      600      +17     
- Partials      164      175      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rjbrown2
Copy link
Contributor Author

rjbrown2 commented Mar 27, 2024

FYI: Currently: (Previous CC | New CC)

crypto_tc.c:

Crypto_TC_ApplySecurity_Cam: (87 | 14)
Crypto_TC_ProcessSecurity_Cam: (64 | 14)

sadb_routine_inmemory_template.c:

sadb_get_operational_sa_from_gvcid: (42 | 2)

crypto.c:

Crypto_Check_Anti_Replay: (24 | 11)

cryptography_interface_libgcrypt_template.c:

cryptography_aead_encrypt: (21 | 12)
cryptography_validate_authentication: (18 | 15)
cryptography_aead_decrypt: (16 | 14)

base64url.c

base64urlDecode: (17 | 13)

standalone.c:

crypto_standalone_tm_process: (17 | 11)

crypto_error.c:

Crypto_Get_Error_Code_Enum_String: (16 | 8)

All new functions added to reduce complexity are also below a score of 15.

@rjbrown2 rjbrown2 merged commit 0770fd9 into dev Mar 28, 2024
5 checks passed
@jlucas9 jlucas9 deleted the 196-reducing-cyclomatic-complexity branch April 4, 2024 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants