Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Fix #2375, Use size_t for variables/parameters representing size #2376

Merged
merged 1 commit into from
Jul 2, 2024
Merged

Fix #2375, Use size_t for variables/parameters representing size #2376

merged 1 commit into from
Jul 2, 2024

Conversation

thnkslprpt
Copy link
Contributor

Checklist

Describe the contribution

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

Expected behavior changes
Some signed variables changed to unsigned but it would have been illogical/impossible to input negative values in those cases.

Contributor Info
Avi Weiss @thnkslprpt

github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 15, 2023
View reviewed changes
modules/core_private/ut-stubs/src/ut_support.c
@@ -507,7 +507,7 @@
** But on the upside this concession avoids a far more complicated issue of
** needing a fully-fledged implementation of printf in the OS_printf stub.
*/
static int UT_StrCmpFormatStr(const char *FormatStr, const char *TestStr, uint32 FormatLength, uint32 TestLength)
static int UT_StrCmpFormatStr(const char *FormatStr, const char *TestStr, size_t FormatLength, size_t TestLength)

Check notice

Code scanning / CodeQL

Function too long

UT_StrCmpFormatStr has too many lines (95, while 60 are allowed).
modules/core_private/ut-stubs/src/ut_support.c
@@ -351,12 +351,12 @@
/*
** Set the CDS size returned by the BSP
*/
uint8 *UT_SetCDSSize(int32 Size)
uint8 *UT_SetCDSSize(size_t Size)

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
modules/core_private/ut-stubs/src/ut_support.c
@@ -334,7 +334,7 @@
/*
** Set the size of the ES reset area
*/
void UT_SetSizeofESResetArea(int32 Size)
void UT_SetSizeofESResetArea(size_t Size)

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
modules/core_private/ut-stubs/src/ut_support.c
@@ -507,7 +507,7 @@
** But on the upside this concession avoids a far more complicated issue of
** needing a fully-fledged implementation of printf in the OS_printf stub.
*/
static int UT_StrCmpFormatStr(const char *FormatStr, const char *TestStr, uint32 FormatLength, uint32 TestLength)
static int UT_StrCmpFormatStr(const char *FormatStr, const char *TestStr, size_t FormatLength, size_t TestLength)

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
modules/es/ut-coverage/es_UT.c
@@ -460,7 +460,7 @@
}
}

void ES_UT_SetupCDSGlobal(uint32 CDS_Size)
void ES_UT_SetupCDSGlobal(size_t CDS_Size)

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
modules/evs/ut-coverage/evs_UT.c
@@ -155,7 +155,7 @@
return StubRetcode;
}

static void UT_EVS_DoDispatchCheckEvents_Impl(void *MsgPtr, uint32 MsgSize, UT_TaskPipeDispatchId_t DispatchId,
static void UT_EVS_DoDispatchCheckEvents_Impl(void *MsgPtr, size_t MsgSize, UT_TaskPipeDispatchId_t DispatchId,

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
zbarnett
zbarnett reviewed Feb 16, 2024
View reviewed changes
modules/es/fsw/src/cfe_es_task.c
@@ -1088,8 +1088,8 @@ int32 CFE_ES_QueryAllCmd(const CFE_ES_QueryAllCmd_t *data)
OS_close(FileDescriptor);
CFE_ES_Global.TaskData.CommandCounter++;
CFE_EVS_SendEvent(CFE_ES_ALL_APPS_EID, CFE_EVS_EventType_DEBUG,
"App Info file written to %s, Entries=%d, FileSize=%d", QueryAllFilename, (int)EntryCount,
(int)FileSize);
"App Info file written to %s, Entries=%d, FileSize=%lu", QueryAllFilename, (int)EntryCount,
Copy link

Choose a reason for hiding this comment

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

This could be %zu and then you wouldn't have to cast FileSize at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

Problem is not all "c99" compilers recognize %zu yet.... Once we update to C11 then we should be able to use the z modifier. For now I prefer to keep the cast to unsigned long.

jphickey
jphickey approved these changes May 23, 2024
View reviewed changes
modules/es/fsw/src/cfe_es_task.c
@@ -1088,8 +1088,8 @@ int32 CFE_ES_QueryAllCmd(const CFE_ES_QueryAllCmd_t *data)
OS_close(FileDescriptor);
CFE_ES_Global.TaskData.CommandCounter++;
CFE_EVS_SendEvent(CFE_ES_ALL_APPS_EID, CFE_EVS_EventType_DEBUG,
"App Info file written to %s, Entries=%d, FileSize=%d", QueryAllFilename, (int)EntryCount,
(int)FileSize);
"App Info file written to %s, Entries=%d, FileSize=%lu", QueryAllFilename, (int)EntryCount,
Copy link
Contributor

Choose a reason for hiding this comment

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

Problem is not all "c99" compilers recognize %zu yet.... Once we update to C11 then we should be able to use the z modifier. For now I prefer to keep the cast to unsigned long.

@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label May 23, 2024
@dzbaker dzbaker added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels May 30, 2024
@dzbaker dzbaker mentioned this pull request Jul 1, 2024
Merged
2 tasks
dzbaker added a commit to nasa/cFS that referenced this pull request Jul 2, 2024
@dzbaker
IC: equuleus-rc1+dev13
4fd8567 
*Combines:*

cFE equuleus-rc1+dev137
osal equuleus-rc1+dev66
elf2cfetbl equuleus-rc1+dev56

**Includes:**

*cFS*
- #707
- #773

*cFE*
- nasa/cFE#2560
- nasa/cFE#2344
- nasa/cFE#2359
- nasa/cFE#2376
- nasa/cFE#2496
- nasa/cFE#2554
- nasa/cFE#2568
- nasa/cFE#2566

*osal*
- nasa/osal#1456
- nasa/osal#1465

*elf2cfetbl*
- nasa/elf2cfetbl#147
- nasa/elf2cfetbl#124
- nasa/elf2cfetbl#125
- nasa/elf2cfetbl#127

Co-authored by: Avi Weiss <thnkslprpt@users.noreply.github.com>
Co-authored by: Justin Figueroa <chillfig@users.noreply.github.com>
Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored by: Anh Van <avan989@users.noreply.github.com>
@dzbaker dzbaker mentioned this pull request Jul 2, 2024
Merged
2 tasks
dzbaker added a commit to nasa/cFS that referenced this pull request Jul 2, 2024
@dzbaker
IC: equuleus-rc1+dev13
ac6463a 
*Combines:*

cFE equuleus-rc1+dev137
osal equuleus-rc1+dev66
elf2cfetbl equuleus-rc1+dev56

**Includes:**

*cFS*
- #707
- #773

*cFE*
- nasa/cFE#2560
- nasa/cFE#2344
- nasa/cFE#2359
- nasa/cFE#2376
- nasa/cFE#2496
- nasa/cFE#2554
- nasa/cFE#2568
- nasa/cFE#2566

*osal*
- nasa/osal#1456
- nasa/osal#1465

*elf2cfetbl*
- nasa/elf2cfetbl#147
- nasa/elf2cfetbl#124
- nasa/elf2cfetbl#125
- nasa/elf2cfetbl#127

Co-authored by: Avi Weiss <thnkslprpt@users.noreply.github.com>
Co-authored by: Justin Figueroa <chillfig@users.noreply.github.com>
Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored by: Anh Van <avan989@users.noreply.github.com>
@dzbaker dzbaker merged commit 6059459 into nasa:main Jul 2, 2024
dzbaker added a commit to nasa/cFS that referenced this pull request Jul 2, 2024
@dzbaker
IC: equuleus-rc1+dev13
9468344 
*Combines:*

cFE equuleus-rc1+dev137
osal equuleus-rc1+dev66
elf2cfetbl equuleus-rc1+dev56

**Includes:**

*cFS*
- #707
- #773

*cFE*
- nasa/cFE#2560
- nasa/cFE#2344
- nasa/cFE#2359
- nasa/cFE#2376
- nasa/cFE#2496
- nasa/cFE#2554
- nasa/cFE#2568
- nasa/cFE#2566

*osal*
- nasa/osal#1456
- nasa/osal#1465

*elf2cfetbl*
- nasa/elf2cfetbl#147
- nasa/elf2cfetbl#124
- nasa/elf2cfetbl#125
- nasa/elf2cfetbl#127

Co-authored by: Avi Weiss <thnkslprpt@users.noreply.github.com>
Co-authored by: Justin Figueroa <chillfig@users.noreply.github.com>
Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored by: Anh Van <avan989@users.noreply.github.com>
@thnkslprpt thnkslprpt deleted the fix-2375-use-size-t-for-size-variables-and-parameters branch July 3, 2024 05:03
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@zbarnett zbarnett zbarnett left review comments

@jphickey jphickey jphickey approved these changes

Assignees
No one assigned
Labels
CCB:Approved Indicates code review and approval by community CCB enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use size_t for variables/parameters representing size
5 participants
@thnkslprpt @zbarnett @jphickey @dzbaker @chillfig