Fix #265, change variable names to be more informative #332
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL-coding-standard found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
| { | ||
| /* no more messages this wakeup allowed */ | ||
| c->cur = t; /* remember where we were for next time */ | ||
| success = false; | ||
| chan->cur = txn; /* remember where we were for next time */ |
Check warning
Code scanning / CodeQL-security
Local variable address stored in non-local memory Warning
| } | ||
|
|
||
| /* don't need CF_CRC_Start() since taken care of by reset_cfdp() */ | ||
| /*CF_CRC_Start(&t->crc);*/ | ||
| /*CF_CRC_Start(&txn->crc);*/ |
Check notice
Code scanning / CodeQL-security
Commented-out code Note
havencarlson
force-pushed
the
fix#265
branch
3 times, most recently
from
043ce89
to
b0719e8
Compare
havencarlson
added
the
CCB:Ready
dzbaker
added
CCB:Approved
and removed
CCB:Ready
|
CCB 6 October 2022: Approved pending merge conflict resolution |
havencarlson
force-pushed
the
fix#265
branch
4 times, most recently
from
7521095
to
c840f27
Compare
dzbaker
added
CCB:Ready
havencarlson
force-pushed
the
fix#265
branch
from
c840f27
to
ea43036
Compare
havencarlson
force-pushed
the
fix#265
branch
2 times, most recently
from
76ad4ad
to
175f863
Compare
| @@ -106,7 +106,7 @@ | |||
|
|
|||
| if (!CF_AppData.engine.out.msg) | |||
| { | |||
| c->cur = t; /* remember where we were for next time */ | |||
| chan->cur = txn; /* remember where we were for next time */ | |||
Check warning
Code scanning / CodeQL-security
Local variable address stored in non-local memory Warning
havencarlson
force-pushed
the
fix#265
branch
from
175f863
to
c2725b1
Compare
dzbaker
added
CCB:Provisionally-Approved
and removed
CCB:Ready
There was a problem hiding this comment.
@havencarlson -- This looked great, thanks for the effort in doing this. I added a new commit to the PR to fixup a few things I saw when reviewing. Mostly - I ran it through clang-format again to fixup some of the white space that got changed due to the difference in name length.
There were a couple other minor updates - There was one function that was still using "pt" as a transaction instead of "txn" so I made it consistent.
I just committed my updates as a second commit, please have a look at 157e04a
Checklist (Please check before submitting)
Describe the contribution
Fix #265, change single-letter variable names to be more descriptive
Testing performed
Unit testing
Expected behavior changes
no impact to behavior
System(s) tested on
Contributor Info - All information REQUIRED for consideration of pull request
Haven Carlson - NASA