uNetworkInterfaceUp: pCfg is not optional for U_NETWORK_TYPE_CELL

[jraats]

Hi RobMeades, After updating to the latest version (master) I moved to the Zephyr device tree implementation which is a great improvement! But after removing all the config structs in my code and using the device tree configuration I came across a fatal error.

My old implementation was:

static const uNetworkCfgCell_t gNetworkCfg = {
    .type = U_NETWORK_TYPE_CELL,
    .pApn = MODEM_DRIVER_APN,
    .timeoutSeconds = MODEM_DRIVER_TIMEOUT_SECONDS
};

int32_t status = uNetworkInterfaceUp(ctx->devHandle, U_NETWORK_TYPE_CELL, &gNetworkCfg);

whereby my new implementation is (param is NULL, because the settings are in the device tree):

int32_t status = uNetworkInterfaceUp(ctx->devHandle, U_NETWORK_TYPE_CELL, NULL);

This change works for U_NETWORK_TYPE_GNSS, but not for U_NETWORK_TYPE_CELL.

After digging in the code I came across this piece of code https://github.com/u-blox/ubxlib/blob/c25c7b30f921c344d7506ebc4bd0eef392a4adc0/common/network/src/u_network.c#L164:

if ((pNetworkData->pCfg != NULL) &&
    (pNetworkData->networkType == (int32_t) U_NETWORK_TYPE_CELL)) {
    // Cellular has the pUartPpp bit also
    pCellUartPppSrc = ((uNetworkCfgCell_t *) pCfg)->pUartPpp;
    pCellUartPppDest = ((uNetworkCfgCell_t *) pNetworkData->pCfg)->pUartPpp;
    if ((pCellUartPppDest != NULL) && (pCellUartPppSrc == NULL)) {
        // If the old network configuration had a PPP UART and the
        // new one doesn't, free the memory we had
        uPortFree((void *) pCellUartPppDest);
        pCellUartPppDest = NULL;
    }
    if ((pCellUartPppSrc != NULL) && (pCellUartPppDest == NULL)) {
        // Need to allocate memory
        pCellUartPppDest = (const uDeviceCfgUart_t *) pUPortMalloc(sizeof(*pCellUartPppDest));
        if (pCellUartPppDest != NULL) {
            memset((void *) pCellUartPppDest, 0, sizeof(*pCellUartPppDest));
        } else {
            // Clean up on error
            uPortFree(pNetworkData->pCfg);
            pNetworkData->pCfg = NULL;
        }
    }
}

which in case of U_NETWORK_TYPE_CELL tries to copy the pUartPpp data. However because I've passed NULL to this function there is no pUartPpp data.

In my opinion pCfg can be optional, because it is now possible to use the device tree. After applying this fix:

if ((pNetworkData->pCfg != NULL) &&
        (pNetworkData->networkType == (int32_t) U_NETWORK_TYPE_CELL) {

to

if ((pNetworkData->pCfg != NULL) &&
        (pNetworkData->networkType == (int32_t) U_NETWORK_TYPE_CELL) && pCfg != NULL) {

the code is working again.

What do you think? Am I correct that pCfg can be optional and is this fix something we want upstream?

Thanks in advance.

[RobMeades]

Ah, yes, it was my intention that it should be possible to have a NULL network configuration for the device tree case, then I forgot about that when adding the PPP stuff. Will fix.

[RobMeades]

Fix in commit 477c03ddb01accff89e6a4eef5281f3dc3aeed4e; will close this now, please feel free to re-open if there is more to discuss or a further issue related to this issue.