fearless_fool
fearless_fool

Reputation: 35229

C "pass by pointer" value not updated when expected

I have a function where the data value is updated by a library call and returned via pointer. Note that in the body of the function, I print the de-referenced value of data following the call to I2C_Transfer()

static bool read_reg(uint8_t reg, uint8_t *data) {
    bool success = I2C_Transfer(appData.drvI2CHandle,
                                APP_OV2640_SENSOR_I2C_ADDR,
                                (void *)&reg, sizeof(reg),
                                (void *)data, sizeof(uint8_t));
    printf("after I2C_Transfer(%02x), *data = 0x%02x\r\n", reg, *data);
    return success;
}

The call to read_reg() looks like this:

uint8_t vid = 0x55;  // pre-initialize vid for debugging
printf("before read_reg(), vid = 0x%02x\r\n", vid);
read_reg(0x0a, &vid);
printf("after read_reg(), vid = 0x%02x\r\n", vid);

When I run it, I see the following:

before read_reg(), vid = 0x55          // <= as expected
after I2C_Transfer(0a), *data = 0x55   // <= expected *data = 0x26
after read_reg(), vid = 0x26           // <= as expected

Note that vid = 0x26 is the expected result.

The question: Why does printf("after I2C_Transfer()") print 0x55 rather than 0x26? It is almost as if the compiler doesn't know that I2C_Transfer() updated *data by reference and decided to print the cached value.

Update III: it's a vendor library bug

Despite the fact that the SPI_Transfer() function is documented as a blocking function, there's strong evidence (which I have yet to prove) that the function is returning before the last data byte is received. And since interrupts are still enabled, the interrupt routine updates the data register after the function has returned.

So: nothing to see here. Move along...

I move to close this entry.

Update II: stubbing I2C_Transfer()

When I stub I2C_Transfer() with the following, it works as expected:

static bool I2X_TransferStub(const DRV_HANDLE handle, uint16_t address,
                         void *const writeBuffer, const size_t writeSize,
                         void *const readBuffer, const size_t readSize) {
    uint8_t *rx_buf = (void *const)readBuffer;
    *rx_buf = 0x26;
    return true;
}

And the output looks like this:

I2C_TransferStub(0x0a) on entry, data = 55
I2C_TransferStub(0x0a) => 26, success = 1

So what is surprising to me is that in a bare-metal environment (no OS) that the value of *data is somehow updated after the call to I2C_Transfer() returns.

Update: source code for I2C_Transfer

Commenters have requested the source code for I2C_Transfer. Its real name is DRV_I2C_WriteReadTransfer(), shown here. It's worth pointing out that this is a synchronous function: it doesn't return until a value is produced.

bool DRV_I2C_WriteReadTransfer(const DRV_HANDLE handle, uint16_t address,
                               void *const writeBuffer, const size_t writeSize,
                               void *const readBuffer, const size_t readSize) {
    return lDRV_I2C_WriteReadTransfer(handle, address, writeBuffer, writeSize,
                                      readBuffer, readSize,
                                      DRV_I2C_TRANSFER_OBJ_FLAG_WR_RD);
}

static bool lDRV_I2C_WriteReadTransfer (
    const DRV_HANDLE handle,
    uint16_t address,
    void* const writeBuffer,
    const size_t writeSize,
    void* const readBuffer,
    const size_t readSize,
    DRV_I2C_TRANSFER_OBJ_FLAGS transferFlags
)
{
    DRV_I2C_CLIENT_OBJ* clientObj = (DRV_I2C_CLIENT_OBJ *)NULL;
    DRV_I2C_OBJ* hDriver = (DRV_I2C_OBJ*)NULL;
    bool isSuccess = false;
    bool isReqAccepted = false;

    /* Validate the driver handle */
    clientObj = lDRV_I2C_DriverHandleValidate(handle);

    if (clientObj == NULL)
    {
        return isSuccess;
    }

    if (transferFlags == DRV_I2C_TRANSFER_OBJ_FLAG_RD)
    {
        if((readSize == 0U) || (readBuffer == NULL))
        {
            return isSuccess;
        }
    }
    else if ((transferFlags == DRV_I2C_TRANSFER_OBJ_FLAG_WR) || (transferFlags == DRV_I2C_TRANSFER_OBJ_FLAG_WR_FRCD))
    {
        if((writeSize == 0U) || (writeBuffer == NULL))
        {
            return isSuccess;
        }
    }
    else
    {
        if((writeSize == 0U) || (writeBuffer == NULL) || (readSize == 0U) || (readBuffer == NULL))
        {
            return isSuccess;
        }
    }

    hDriver = clientObj->hDriver;


    /* Block other threads from accessing the PLIB */
    if (OSAL_MUTEX_Lock(&hDriver->transferMutex, OSAL_WAIT_FOREVER ) == OSAL_RESULT_SUCCESS)
    {
        /* Error is cleared for every new transfer */
        clientObj->errors = DRV_I2C_ERROR_NONE;

        /* Errors if any, will be saved in the activeClient in the driver callback */
        hDriver->activeClient = (uintptr_t)clientObj;

        /* Check if the transfer setup for this client is different than the current transfer setup */
        if (hDriver->currentTransferSetup.clockSpeed != clientObj->transferSetup.clockSpeed)
        {
            /* Set the new transfer setup */
            (void) hDriver->i2cPlib->transferSetup(&clientObj->transferSetup, 0);

            hDriver->currentTransferSetup.clockSpeed = clientObj->transferSetup.clockSpeed;
        }

        switch(transferFlags)
        {
            case DRV_I2C_TRANSFER_OBJ_FLAG_RD:
                if (hDriver->i2cPlib->read_t(address, readBuffer, readSize) == true)
                {
                    isReqAccepted = true;
                }
                break;

            case DRV_I2C_TRANSFER_OBJ_FLAG_WR:
                if (hDriver->i2cPlib->write_t(address, writeBuffer, writeSize) == true)
                {
                    isReqAccepted = true;
                }
                break;


            case DRV_I2C_TRANSFER_OBJ_FLAG_WR_RD:
                if (hDriver->i2cPlib->writeRead(address, writeBuffer, writeSize, readBuffer, readSize) == true)
                {
                    isReqAccepted = true;
                }
                break;

            default:
                     /* Nothing to do */
                break;
        }

        if (isReqAccepted == true)
        {
            /* Wait till transfer completes. This semaphore is released from ISR */
            if (OSAL_SEM_Pend( &hDriver->transferDone, OSAL_WAIT_FOREVER ) == OSAL_RESULT_SUCCESS)
            {
                if (hDriver->transferStatus == DRV_I2C_TRANSFER_STATUS_COMPLETE)
                {
                    isSuccess = true;
                }
            }
        }
        else
        {
            /* Update error into the client object*/
            clientObj->errors = hDriver->i2cPlib->errorGet();

            if(clientObj->errors == DRV_I2C_ERROR_NONE)
            {
                hDriver->transferStatus = DRV_I2C_TRANSFER_STATUS_COMPLETE;
            }
            else
            {
                hDriver->transferStatus = DRV_I2C_TRANSFER_STATUS_ERROR;
            }
        }

        /* Release the mutex to allow other threads to access the PLIB */
        (void) OSAL_MUTEX_Unlock(&hDriver->transferMutex);
    }

    return isSuccess;
}

Upvotes: 2

Views: 116

Answers (0)

Related Questions