Skip to content
Merged
Changes from all commits
Commits
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
53 changes: 50 additions & 3 deletions stream_buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,11 @@ size_t xStreamBufferSend( StreamBufferHandle_t xStreamBuffer,
size_t xReturn, xSpace = 0;
size_t xRequiredSpace = xDataLengthBytes;
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( pxStreamBuffer );

Expand All @@ -532,13 +536,56 @@ size_t xStreamBufferSend( StreamBufferHandle_t xStreamBuffer,

/* Overflow? */
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
{
/* 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
{
mtCOVERAGE_TEST_MARKER();
/* 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 );

Expand Down