SOCKETS_Send is wrongly used (or wrongly documented) by IoT lib

[lightblu]

Describe the bug SOCKETS_Send is wrongly used (or wrongly documented); returning less than the expected number of bytes is treated as error in some places.

https://github.com/aws/amazon-freertos/blob/master/libraries/abstractions/secure_sockets/include/iot_secure_sockets.h documents

/**
 * @brief Transmit data to the remote socket.
 *
 * The socket must have already been created using a call to SOCKETS_Socket() and
 * connected to a remote socket using SOCKETS_Connect().
 *
 * See the [Berkeley Sockets API]
 * (https://en.wikipedia.org/wiki/Berkeley_sockets#Socket_API_functions)
 * in wikipedia
 *
 * @param[in] xSocket The handle of the sending socket.
 * @param[in] pvBuffer The buffer containing the data to be sent.
 * @param[in] xDataLength The length of the data to be sent.
 * @param[in] ulFlags Not currently used. Should be set to 0.
 *
 * @return
 * * On success, the number of bytes actually sent is returned.
 * * If an error occurred, a negative value is returned. @ref SocketsErrors
 */
/* @[declare_secure_sockets_send] */
int32_t SOCKETS_Send( Socket_t xSocket,
                      const void * pvBuffer,
                      size_t xDataLength,
                      uint32_t ulFlags );

"On success, the number of bytes actually sent is returned." in my understanding also allows only partial sends, so returning a number of bytes > 0 but < xDataLength. Coming from an old version ( https://github.com/aws/amazon-freertos/blob/master/CHANGELOG.md#v148-05212019 ), this is also how it worked fine there. If you returned less bytes, SOCKETS_Send was called with the remaining bytes. This pattern is also still present in source files like https://github.com/aws/amazon-freertos/blob/master/libraries/freertos_plus/aws/greengrass/src/aws_helper_secure_connect.c :

        while( ulBytesSent < ulPayloadSize )
        {
            /* Try sending the remaining data. */
            lSendRetVal = SOCKETS_Send( xSocket,
                                        ( const unsigned char * ) &pcPayload[ ulBytesSent ],
                                        ( size_t ) ( ulPayloadSize - ulBytesSent ),
                                        ( uint32_t ) 0 );

            /* A negative return value from SOCKETS_Send
             * means some error occurred. */
            if( lSendRetVal < 0 )
            {
                break;
            }
            else
            {
                /* Update the count of sent bytes. */
                ulBytesSent += ( uint32_t ) lSendRetVal;
            }
}```

however in a other places (also tests) sending less than xDataLength is treated/expected now as error case. E.g.

https://github.com/aws/amazon-freertos/blob/master/libraries/abstractions/platform/freertos/iot_network_freertos.c
```cpp
    return status;
}

/*-----------------------------------------------------------*/

size_t IotNetworkAfr_Send( void * pConnection,
                           const uint8_t * pMessage,
                           size_t messageLength )
{
    size_t bytesSent = 0;
    int32_t socketStatus = SOCKETS_ERROR_NONE;

    /* Cast network connection to the correct type. */
    _networkConnection_t * pNetworkConnection = ( _networkConnection_t * ) pConnection;

    /* Only one thread at a time may send on the connection. Lock the socket
     * mutex to prevent other threads from sending. */
    if( xSemaphoreTake( ( QueueHandle_t ) &( pNetworkConnection->socketMutex ),
                        portMAX_DELAY ) == pdTRUE )
    {
        socketStatus = SOCKETS_Send( pNetworkConnection->socket,
                                     pMessage,
                                     messageLength,
                                     0 );

        if( socketStatus > 0 )
        {
            bytesSent = ( size_t ) socketStatus;
        }
        else
        {
            IotLogError( "Error %ld while sending data.", ( long int ) socketStatus );
        }

        xSemaphoreGive( ( QueueHandle_t ) &( pNetworkConnection->socketMutex ) );
    }

    return bytesSent;
}

used by (and treated as error if not all bytes ahve been transmitted) in https://github.com/aws/amazon-freertos/blob/master/libraries/c_sdk/standard/mqtt/src/iot_mqtt_operation.c

        /* Transmit the MQTT packet from the operation over the network. */
        bytesSent = pMqttConnection->pNetworkInterface->send( pMqttConnection->pNetworkConnection,
                                                              pOperation->u.operation.pMqttPacket,
                                                              pOperation->u.operation.packetSize );

        /* Check transmission status. */
        if( bytesSent != pOperation->u.operation.packetSize )
        {
            pOperation->u.operation.status = IOT_MQTT_NETWORK_ERROR;
        }
        else

Expected behavior

SOCKETS_Send should be repeatedly called until all data is sent in all places using it, or SOCKETS_Send interface documentation be changed.

[yourslab]

Thank you for bringing this up. We are currently working on a fix to loop until all data is sent.

[yourslab]

Commit 63856c1626f7 fixes this issue. I will close this now. Thank you.