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

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

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

@@ -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).
@@ -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.
@@ -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.
@@ -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.
@@ -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.
@@ -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.
@@ -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,

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.

@@ -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 added a commit to nasa/cFS that referenced this pull request Jul 2, 2024
*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 <[email protected]>
Co-authored by: Justin Figueroa <[email protected]>
Co-authored by: Joseph Hickey <[email protected]>
Co-authored by: Jacob Hageman <[email protected]>
Co-authored by: Anh Van <[email protected]>
@dzbaker dzbaker mentioned this pull request Jul 2, 2024
2 tasks
dzbaker added a commit to nasa/cFS that referenced this pull request Jul 2, 2024
*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 <[email protected]>
Co-authored by: Justin Figueroa <[email protected]>
Co-authored by: Joseph Hickey <[email protected]>
Co-authored by: Jacob Hageman <[email protected]>
Co-authored by: Anh Van <[email protected]>
@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
*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 <[email protected]>
Co-authored by: Justin Figueroa <[email protected]>
Co-authored by: Joseph Hickey <[email protected]>
Co-authored by: Jacob Hageman <[email protected]>
Co-authored by: Anh Van <[email protected]>
@thnkslprpt thnkslprpt deleted the fix-2375-use-size-t-for-size-variables-and-parameters branch July 3, 2024 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use size_t for variables/parameters representing size
5 participants