Skip to content

Warning fixes. #356

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

Merged
merged 6 commits into from
Jul 20, 2021
Merged

Warning fixes. #356

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a1503ef
Warning fixes.
kjassmann-renesas Jun 18, 2021
943cc9b
Merge remote-tracking branch 'origin/main' into tasks-timers-warning-fix
kjassmann-renesas Jul 20, 2021
c124148
Revert change to tasks.c.
kjassmann-renesas Jul 20, 2021
1cc4d97
Remove all empty definitions of portCLEAN_UP_TCB( pxTCB ) and portALL…
kjassmann-renesas Jul 20, 2021
2d1f575
Use cast to fix warnings.
kjassmann-renesas Jul 20, 2021
cf557a1
Coding standard fix.
kjassmann-renesas Jul 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions tasks.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3945,6 +3945,10 @@ static void prvCheckTasksWaitingTermination( void )
* want to allocate and clean RAM statically. */
portCLEAN_UP_TCB( pxTCB );

/* Remove compiler warning about unused variables when portCLEAN_UP_TCB
* is not being used. */
( void ) pxTCB;
Copy link
Contributor

@mingyue86010 mingyue86010 Jun 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
( void ) pxTCB;
#if !defined( portCLEAN_UP_TCB ) || ( portCLEAN_UP_TCB == 0 )
( void ) pxTCB;
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the review.

This won't work because portCLEAN_UP_TCB is defined. It's just defined to nothing.

        #define portCLEAN_UP_TCB( pxTCB )

Perhaps the comment wording could be improved. I'm open to suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah thank you for pointing out. I updated the changes here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The updated changes also will not work. portCLEAN_UP_TCB is defined to nothing, it is not defined to 0.

Copy link
Member

Choose a reason for hiding this comment

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

Does this default definition not take care of this? https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/include/FreeRTOS.h#L314

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would, except that portCLEAN_UP_TCB is already defined in many ports provided in the repository. Examples:

https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/portable/ARMv8M/non_secure/portable/GCC/ARM_CM33/portmacro.h#L269

https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/portable/IAR/ARM_CM33/non_secure/portmacro.h#L269

I could fix/remove these instances of portCLEAN_UP_TCB instead if you prefer.


/* Free up the memory allocated by the scheduler for the task. It is up
* to the task to free any memory allocated at the application level.
* See the third party link http://www.nadler.com/embedded/newlibAndFreeRTOS.html
Expand Down
17 changes: 10 additions & 7 deletions timers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,12 @@
#endif

/* Bit definitions used in the ucStatus member of a timer structure. */
#define tmrSTATUS_IS_ACTIVE ( ( uint8_t ) 0x01 )
#define tmrSTATUS_IS_STATICALLY_ALLOCATED ( ( uint8_t ) 0x02 )
#define tmrSTATUS_IS_AUTORELOAD ( ( uint8_t ) 0x04 )
#define tmrSTATUS_IS_ACTIVE ( ( uint8_t ) 0x01 )
#define tmrSTATUS_IS_STATICALLY_ALLOCATED ( ( uint8_t ) 0x02 )
#define tmrSTATUS_IS_AUTORELOAD ( ( uint8_t ) 0x04 )
#define tmrSTATUS_CLEAR_ACTIVE ( ( uint8_t ) ~0x01 )
#define tmrSTATUS_CLEAR_STATICALLY_ALLOCATED ( ( uint8_t ) ~0x02 )
#define tmrSTATUS_CLEAR_AUTORELOAD ( ( uint8_t ) ~0x04 )
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#define tmrSTATUS_CLEAR_ACTIVE ( ( uint8_t ) ~0x01 )
#define tmrSTATUS_CLEAR_STATICALLY_ALLOCATED ( ( uint8_t ) ~0x02 )
#define tmrSTATUS_CLEAR_AUTORELOAD ( ( uint8_t ) ~0x04 )
#define tmrSTATUS_CLEAR_ACTIVE ( ( uint8_t ) ~tmrSTATUS_IS_ACTIVE )
#define tmrSTATUS_CLEAR_STATICALLY_ALLOCATED ( ( uint8_t ) ~tmrSTATUS_IS_STATICALLY_ALLOCATED )
#define tmrSTATUS_CLEAR_AUTORELOAD ( ( uint8_t ) ~tmrSTATUS_IS_AUTORELOAD )

Copy link
Member

Choose a reason for hiding this comment

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

The above is just a suggestion if we decide to add new defines.


/* The definition of the timers themselves. */
typedef struct tmrTimerControl /* The old naming convention is used to prevent breaking kernel aware debuggers. */
Expand Down Expand Up @@ -462,7 +465,7 @@
}
else
{
pxTimer->ucStatus &= ~tmrSTATUS_IS_AUTORELOAD;
pxTimer->ucStatus &= tmrSTATUS_CLEAR_AUTORELOAD;
}
}
taskEXIT_CRITICAL();
Expand Down Expand Up @@ -550,7 +553,7 @@
}
else
{
pxTimer->ucStatus &= ~tmrSTATUS_IS_ACTIVE;
pxTimer->ucStatus &= tmrSTATUS_CLEAR_ACTIVE;
}

/* Call the timer callback. */
Expand Down Expand Up @@ -846,7 +849,7 @@
case tmrCOMMAND_STOP:
case tmrCOMMAND_STOP_FROM_ISR:
/* The timer has already been removed from the active list. */
pxTimer->ucStatus &= ~tmrSTATUS_IS_ACTIVE;
pxTimer->ucStatus &= tmrSTATUS_CLEAR_ACTIVE;
break;

case tmrCOMMAND_CHANGE_PERIOD:
Expand Down Expand Up @@ -885,7 +888,7 @@
* could not have been dynamically allocated. So there is
* no need to free the memory - just mark the timer as
* "not active". */
pxTimer->ucStatus &= ~tmrSTATUS_IS_ACTIVE;
pxTimer->ucStatus &= tmrSTATUS_CLEAR_ACTIVE;
}
#endif /* configSUPPORT_DYNAMIC_ALLOCATION */
break;
Expand Down