No functional changes. (#194)

Shorted overly verbose and opinionated comments in xStreamBufferSend().
Remove the unnecessary xIsFeasible variable from xStreamBufferSend().
This commit is contained in:
RichardBarry 2020-10-03 21:26:22 -07:00 committed by GitHub
parent b1307dbea8
commit fccb97b10b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -518,10 +518,6 @@ 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 );
@ -536,55 +532,35 @@ 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 /* If this is a message buffer then it must be possible to write the
* a stream buffer for semantic reasons. Check if it is physically possible to write the message given * whole message. */
* the length of the buffer. */
if( xRequiredSpace > pxStreamBuffer->xLength ) if( xRequiredSpace > pxStreamBuffer->xLength )
{ {
/* The message could never be written because it is greater than the buffer length. /* The message would not fit even if the entire buffer was empty,
* By setting xIsFeasable to FALSE, we skip over the following do..while loop, thus avoiding * so don't wait for space. */
* a deadlock. The call to 'prvWriteMessageToBuffer' toward the end of this function with xTicksToWait = ( TickType_t ) 0;
* 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 = pdFALSE;
} }
else else
{ {
/* It is possible to write the message completely in the buffer. This is the intended route. mtCOVERAGE_TEST_MARKER();
* Let's continue with the regular timeout logic. */
xIsFeasible = pdTRUE;
} }
} }
else else
{ {
/* In the case of the stream buffer, not being able to completely write the message in the buffer /* If this is a stream buffer then it is acceptable to write only part
* is an acceptable scenario, but it has to be dealt with properly */ * of the message to the buffer. Cap the length to the total length of
* the buffer. */
if( xRequiredSpace > pxStreamBuffer->xLength ) 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; 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 = pdTRUE;
} }
else else
{ {
/* It is possible to write the message completely in the buffer. */ mtCOVERAGE_TEST_MARKER();
xIsFeasible = pdTRUE;
} }
} }
/* 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 )
if( ( xTicksToWait != ( TickType_t ) 0 ) && ( xIsFeasible == pdTRUE ) )
{ {
vTaskSetTimeOutState( &xTimeOut ); vTaskSetTimeOutState( &xTimeOut );