Skip to content

Add some assertions and coverage exceptions to queue.c #273

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 9 commits into from
Mar 6, 2021
Merged

Add some assertions and coverage exceptions to queue.c #273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
699a7d8
Add an LCOV_BRANCH exception for the check that sizeof( StaticQueue_t…
paulbartell Mar 5, 2021
cc3f74b
Add LCOV_BRANCH coverage exception for a configASSERT on pxQueueSetCo…
paulbartell Mar 5, 2021
58ba569
Add configASSERTs to alert when invalid parameters are passed into Qu…
paulbartell Mar 5, 2021
d1d08e4
Assert that the semaphore handle passed into xQueueGetMutexHolder is …
paulbartell Mar 5, 2021
d4099a1
Correct some typos in queue.c
paulbartell Mar 5, 2021
676ce01
Update lexicon.txt
paulbartell Mar 5, 2021
7203a2e
Merge branch 'main' into pbartell/fixup-queue-assertions
alfred2g Mar 6, 2021
b065256
Remove vQueueAddToRegistry assertion on failure
paulbartell Mar 6, 2021
8bf54a1
Merge branch 'pbartell/fixup-queue-assertions' of github.com:paulbart…
paulbartell Mar 6, 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
6 changes: 4 additions & 2 deletions .github/lexicon.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1529,6 +1529,7 @@ prvinitialisenewstreambuffer
prvinitialisenewtimer
prvinsertblockintofreelist
prvlockqueue
prvnotifyqueuesetcontainer
prvportmalloc
prvportresetpic
prvprocesssimulatedinterrupts
Expand Down Expand Up @@ -1631,7 +1632,6 @@ pvyieldevent
pwdtc
pwm
pwmc
pxtaskcode
pxblock
pxblocktoinsert
pxcallbackfunction
Expand Down Expand Up @@ -1688,6 +1688,7 @@ pxprevious
pxpreviouswaketime
pxqueue
pxqueuebuffer
pxqueuesetcontainer
pxramstack
pxreadycoroutinelists
pxreadytaskslists
Expand All @@ -1707,6 +1708,7 @@ pxstreambuffercreatestatic
pxtagvalue
pxtask
pxtaskbuffer
pxtaskcode
pxtaskdefinition
pxtaskstatus
pxtaskstatusarray
Expand Down Expand Up @@ -2653,7 +2655,6 @@ wu
www
wwwfreertos
wxr
xtasktodelete
xa
xaa
xaaaa
Expand Down Expand Up @@ -3020,6 +3021,7 @@ xtaskswaitingforbits
xtaskswaitingtermination
xtaskswaitingtoreceive
xtaskswaitingtosend
xtasktodelete
xtasktonotify
xtasktoquery
xtasktoresume
Expand Down
30 changes: 22 additions & 8 deletions queue.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,10 @@ BaseType_t xQueueGenericReset( QueueHandle_t xQueue,
* variable of type StaticQueue_t or StaticSemaphore_t equals the size of
* the real queue and semaphore structures. */
volatile size_t xSize = sizeof( StaticQueue_t );
configASSERT( xSize == sizeof( Queue_t ) );
( void ) xSize; /* Keeps lint quiet when configASSERT() is not defined. */

/* This assertion cannot be branch covered in unit tests */
configASSERT( xSize == sizeof( Queue_t ) ); /* LCOV_EXCL_BR_LINE */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is really a job for _Static_assert (C11).

( void ) xSize; /* Keeps lint quiet when configASSERT() is not defined. */
}
#endif /* configASSERT_DEFINED */

Expand Down Expand Up @@ -398,7 +400,7 @@ BaseType_t xQueueGenericReset( QueueHandle_t xQueue,
configASSERT( ( uxItemSize == 0 ) || ( uxQueueLength == ( xQueueSizeInBytes / uxItemSize ) ) );

/* Check for addition overflow. */
configASSERT( ( sizeof( Queue_t ) + xQueueSizeInBytes ) > xQueueSizeInBytes );
configASSERT( ( sizeof( Queue_t ) + xQueueSizeInBytes ) > xQueueSizeInBytes );

/* Allocate the queue and storage area. Justification for MISRA
* deviation as follows: pvPortMalloc() always ensures returned memory
Expand Down Expand Up @@ -561,6 +563,8 @@ static void prvInitialiseNewQueue( const UBaseType_t uxQueueLength,
TaskHandle_t pxReturn;
Queue_t * const pxSemaphore = ( Queue_t * ) xSemaphore;

configASSERT( xSemaphore );

/* This function is called by xSemaphoreGetMutexHolder(), and should not
* be called directly. Note: This is a good way of determining if the
* calling task is the mutex holder, but not a good way of determining the
Expand Down Expand Up @@ -944,15 +948,15 @@ BaseType_t xQueueGenericSend( QueueHandle_t xQueue,
vTaskPlaceOnEventList( &( pxQueue->xTasksWaitingToSend ), xTicksToWait );

/* Unlocking the queue means queue events can effect the
* event list. It is possible that interrupts occurring now
* event list. It is possible that interrupts occurring now
* remove this task from the event list again - but as the
* scheduler is suspended the task will go onto the pending
* ready last instead of the actual ready list. */
* ready list instead of the actual ready list. */
prvUnlockQueue( pxQueue );

/* Resuming the scheduler will move tasks from the pending
* ready list into the ready list - so it is feasible that this
* task is already in a ready list before it yields - in which
* task is already in the ready list before it yields - in which
* case the yield will not cause a context switch unless there
* is also a higher priority task in the pending ready list. */
if( xTaskResumeAll() == pdFALSE )
Expand Down Expand Up @@ -1774,7 +1778,7 @@ BaseType_t xQueuePeek( QueueHandle_t xQueue,
taskEXIT_CRITICAL();

/* Interrupts and other tasks can send to and receive from the queue
* now the critical section has been exited. */
* now that the critical section has been exited. */

vTaskSuspendAll();
prvLockQueue( pxQueue );
Expand Down Expand Up @@ -2723,6 +2727,9 @@ BaseType_t xQueueIsQueueFullFromISR( const QueueHandle_t xQueue )
{
UBaseType_t ux;

configASSERT( xQueue );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change will result in users with configASSERT enabled not being able to add a NULL queue handle to the queue registry.

configASSERT( pcQueueName );

/* See if there is an empty space in the registry. A NULL name denotes
* a free slot. */
for( ux = ( UBaseType_t ) 0U; ux < ( UBaseType_t ) configQUEUE_REGISTRY_SIZE; ux++ )
Expand Down Expand Up @@ -2753,6 +2760,8 @@ BaseType_t xQueueIsQueueFullFromISR( const QueueHandle_t xQueue )
UBaseType_t ux;
const char * pcReturn = NULL; /*lint !e971 Unqualified char types are allowed for strings and single characters only. */

configASSERT( xQueue );

/* Note there is nothing here to protect against another task adding or
* removing entries from the registry while it is being searched. */

Expand Down Expand Up @@ -2781,6 +2790,8 @@ BaseType_t xQueueIsQueueFullFromISR( const QueueHandle_t xQueue )
{
UBaseType_t ux;

configASSERT( xQueue );

/* See if the handle of the queue being unregistered in actually in the
* registry. */
for( ux = ( UBaseType_t ) 0U; ux < ( UBaseType_t ) configQUEUE_REGISTRY_SIZE; ux++ )
Expand Down Expand Up @@ -2967,7 +2978,10 @@ BaseType_t xQueueIsQueueFullFromISR( const QueueHandle_t xQueue )

/* This function must be called form a critical section. */

configASSERT( pxQueueSetContainer );
/* The following line is not reachable in unit tests because every call
* to prvNotifyQueueSetContainer is preceded by a check that
* pxQueueSetContainer != NULL */
configASSERT( pxQueueSetContainer ); /* LCOV_EXCL_BR_LINE */
configASSERT( pxQueueSetContainer->uxMessagesWaiting < pxQueueSetContainer->uxLength );

if( pxQueueSetContainer->uxMessagesWaiting < pxQueueSetContainer->uxLength )
Expand Down