mirror of
https://github.com/FreeRTOS/FreeRTOS-Kernel.git
synced 2025-04-21 22:11:57 -04:00
Update stream_buffer.c (#94)
Add necessary checks when sending data to the stream/message buffer in order to avoid a task deadlock when attempting to write a longer stream/message than the underlying buffer can write.
This commit is contained in:
parent
61635d5b8b
commit
61fc74f0c5
|
@ -519,6 +519,10 @@ size_t xStreamBufferSend( StreamBufferHandle_t xStreamBuffer,
|
||||||
size_t xRequiredSpace = xDataLengthBytes;
|
size_t xRequiredSpace = xDataLengthBytes;
|
||||||
TimeOut_t xTimeOut;
|
TimeOut_t xTimeOut;
|
||||||
|
|
||||||
|
/* Having a 'isFeasible' variable allows to respect the convention that there is only a return statement at the end. Othewise, return
|
||||||
|
* could be done as soon as we realise the send cannot happen. We will let the call to 'prvWriteMessageToBuffer' dealing with this scenario. */
|
||||||
|
BaseType_t xIsFeasible;
|
||||||
|
|
||||||
configASSERT( pvTxData );
|
configASSERT( pvTxData );
|
||||||
configASSERT( pxStreamBuffer );
|
configASSERT( pxStreamBuffer );
|
||||||
|
|
||||||
|
@ -532,13 +536,56 @@ size_t xStreamBufferSend( StreamBufferHandle_t xStreamBuffer,
|
||||||
|
|
||||||
/* Overflow? */
|
/* Overflow? */
|
||||||
configASSERT( xRequiredSpace > xDataLengthBytes );
|
configASSERT( xRequiredSpace > xDataLengthBytes );
|
||||||
|
|
||||||
|
/* In the case of the message buffer, one has to be able to write the complete message as opposed to
|
||||||
|
* a stream buffer for semantic reasons. Check if it is physically possible to write the message given
|
||||||
|
* the length of the buffer. */
|
||||||
|
if(xRequiredSpace > pxStreamBuffer->xLength)
|
||||||
|
{
|
||||||
|
/* The message could never be written because it is greater than the buffer length.
|
||||||
|
* By setting xIsFeasable to FALSE, we skip over the following do..while loop, thus avoiding
|
||||||
|
* a deadlock. The call to 'prvWriteMessageToBuffer' toward the end of this function with
|
||||||
|
* xRequiredSpace greater than xSpace will suffice in not writing anything to the internal buffer.
|
||||||
|
* Now, the function will return 0 because the message could not be written. Should an error code be
|
||||||
|
* returned instead ??? In my opinion, probably.. But the return type doesn't allow for negative
|
||||||
|
* values to be returned. A confusion could exist to the caller. Returning 0 because a timeout occurred
|
||||||
|
* and a subsequent send attempts could eventually succeed, and returning 0 because a write could never
|
||||||
|
* happen because of the size are two scenarios to me :/ */
|
||||||
|
xIsFeasible = FALSE;
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
mtCOVERAGE_TEST_MARKER();
|
/* It is possible to write the message completely in the buffer. This is the intended route.
|
||||||
|
* Let's continue with the regular timeout logic. */
|
||||||
|
xIsFeasible = TRUE;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
/* In the case of the stream buffer, not being able to completely write the message in the buffer
|
||||||
|
* is an acceptable scenario, but it has to be dealt with properly */
|
||||||
|
if(xRequiredSpace > pxStreamBuffer->xLength)
|
||||||
|
{
|
||||||
|
/* Not enough buffer space. We will attempt to write as much as we can in this run
|
||||||
|
* so that the caller can send the remaining in subsequent calls. We avoid a deadlock by
|
||||||
|
* offering the possibility to take the 'else' branch in the 'if( xSpace < xRequiredSpace )'
|
||||||
|
* condition inside the following do..while loop */
|
||||||
|
xRequiredSpace = pxStreamBuffer->xLength;
|
||||||
|
|
||||||
|
/* TODO FIXME: Is there a check we should do with the xTriggerLevelBytes value ? */
|
||||||
|
|
||||||
|
/* With the adjustment to 'xRequiredSpace', the deadlock is avoided, thus it's now feasible. */
|
||||||
|
xIsFeasible = TRUE;
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
/* It is possible to write the message completely in the buffer. */
|
||||||
|
xIsFeasible = TRUE;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if( xTicksToWait != ( TickType_t ) 0 )
|
/* Added check against xIsFeasible. If it's not feasible, don't even wait for notification, let the call to 'prvWriteMessageToBuffer' do nothing and return 0 */
|
||||||
|
if( xTicksToWait != ( TickType_t ) 0 && xIsFeasible == TRUE )
|
||||||
{
|
{
|
||||||
vTaskSetTimeOutState( &xTimeOut );
|
vTaskSetTimeOutState( &xTimeOut );
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue