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.

Sign up for GitHub

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

Jump to bottom

Fix #92, Use size_t for 'size' variables #93

Merged
merged 1 commit into from
Mar 30, 2023
Merged

Fix #92, Use size_t for 'size' variables #93

merged 1 commit into from
Mar 30, 2023

Conversation

thnkslprpt
Copy link
Contributor

Checklist

Describe the contribution
Fixes #92
Easy-to-find size variables converted to size_t type.

All except the local variables in FM_ChildDirListPktCmd() and FM_ChildDirListFileLoop() were already of type uint32, so no real risk of signedness issues popping up.

Note: I did not scrub the entire app - if someone can suggest some additional variables to convert over to size_t I can add them to this PR.

A few typos and spacing issues cleared up in the same files.

Testing performed
GitHub CI actions (incl. Build + Run, Unit Tests etc.) all passing successfully.

Expected behavior changes
Variables representing size should use size_t where possible - more expressive and more compliant with the various (relevant) coding guidelines.

Contributor Info
Avi Weiss @thnkslprpt

github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 10, 2023
View reviewed changes
fsw/src/fm_cmd_utils.c
@@ -158,7 +158,7 @@
}
}

uint32 FM_GetFilenameState(const char *Filename, uint32 BufferSize, bool FileInfoCmd)
uint32 FM_GetFilenameState(const char *Filename, size_t BufferSize, bool FileInfoCmd)

Check notice

Code scanning / CodeQL-coding-standard

Function too long

FM_GetFilenameState has too many lines (78, while 60 are allowed).
fsw/src/fm_cmd_utils.c
@@ -270,7 +270,7 @@
/* */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */

bool FM_VerifyFileState(FM_File_States State, const char *Filename, uint32 BufferSize, uint32 EventID,
bool FM_VerifyFileState(FM_File_States State, const char *Filename, size_t BufferSize, uint32 EventID,

Check notice

Code scanning / CodeQL-coding-standard

Function too long

FM_VerifyFileState has too many lines (103, while 60 are allowed).
fsw/src/fm_cmd_utils.c
@@ -158,7 +158,7 @@
}
}

uint32 FM_GetFilenameState(const char *Filename, uint32 BufferSize, bool FileInfoCmd)
uint32 FM_GetFilenameState(const char *Filename, size_t BufferSize, bool FileInfoCmd)

Check notice

Code scanning / CodeQL-coding-standard

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
fsw/src/fm_cmd_utils.c
@@ -243,7 +243,7 @@
/* */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */

uint32 FM_VerifyNameValid(const char *Name, uint32 BufferSize, uint32 EventID, const char *CmdText)
uint32 FM_VerifyNameValid(const char *Name, size_t BufferSize, uint32 EventID, const char *CmdText)

Check notice

Code scanning / CodeQL-coding-standard

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
fsw/src/fm_cmd_utils.c
@@ -270,7 +270,7 @@
/* */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */

bool FM_VerifyFileState(FM_File_States State, const char *Filename, uint32 BufferSize, uint32 EventID,
bool FM_VerifyFileState(FM_File_States State, const char *Filename, size_t BufferSize, uint32 EventID,

Check notice

Code scanning / CodeQL-coding-standard

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
@thnkslprpt thnkslprpt force-pushed the fix-92-use-size_t-for-sizes branch 4 times, most recently from 8a21666 to fd3a490 Compare March 14, 2023 13:13
@dzbaker dzbaker self-requested a review March 16, 2023 18:40
dzbaker
dzbaker requested changes Mar 16, 2023
View reviewed changes
fsw/src/fm_cmd_utils.h Outdated Show resolved Hide resolved
@thnkslprpt thnkslprpt requested a review from dzbaker March 16, 2023 23:58
@dzbaker
Copy link
Contributor

dzbaker commented Mar 30, 2023

@thnkslprpt CCB 30 March 2023: Approved pending conflict resolution.

@thnkslprpt
Copy link
Contributor Author

@thnkslprpt CCB 30 March 2023: Approved pending conflict resolution.

All done @dzbaker.
Cheers

@dzbaker dzbaker merged commit 7611550 into nasa:main Mar 30, 2023
@thnkslprpt thnkslprpt deleted the fix-92-use-size_t-for-sizes branch March 30, 2023 20:51
@dmknutsen dmknutsen added this to the Equuleus milestone May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dzbaker dzbaker dzbaker approved these changes

Assignees
No one assigned
Labels
CCB:Approved enhancement Equuleus-rc1
Projects
None yet
Milestone
Equuleus
Development

Successfully merging this pull request may close these issues.

Use size_t for 'size' variables
4 participants
@thnkslprpt @dzbaker @dmknutsen @chillfig