Skip to content
This repository has been archived by the owner on Nov 11, 2024. It is now read-only.

Commit

Permalink
Device change only: avoid modifying output parameter on error (#146)
Browse files Browse the repository at this point in the history
Best practice is to avoid modifying output parameters if a function is failing, especially when handles are returned, this supports consistent cleanup code.  The following pattern is now supported for uDeviceOpen():

        uDeviceHandle_t device_handle = NULL;
        int32_t ret;

        if ((ret = uDeviceOpen(&gDeviceCfg, &device_handle)) != 0) {
                uPortLog("ERR: uDeviceOpen %d\n", ret);
                goto cleanup;
        }

    cleanup:

        if (device_handle != NULL) {
                uDeviceClose(device_handle, false);
                device_handle = NULL;
        }

Signed-off-by: Alon Bar-Lev <alon.barlev@gmail.com>
  • Loading branch information
alonbl authored Oct 14, 2023
1 parent eb737d1 commit 7df8ac9
Showing 1 changed file with 14 additions and 8 deletions.
22 changes: 14 additions & 8 deletions common/device/src/u_device.c
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ int32_t uDeviceOpen(const uDeviceCfg_t *pDeviceCfg, uDeviceHandle_t *pDeviceHand
// Lock the API
int32_t errorCode = uDeviceLock();

uDeviceHandle_t deviceHandleCandidate = NULL;

if (errorCode == 0 && pDeviceCfg != NULL) {
errorCode = uDeviceCallback("open", (void *)pDeviceCfg->deviceType, NULL);
}
Expand All @@ -267,27 +269,27 @@ int32_t uDeviceOpen(const uDeviceCfg_t *pDeviceCfg, uDeviceHandle_t *pDeviceHand
if ((pDeviceCfg != NULL) && (pDeviceCfg->version == 0) && (pDeviceHandle != NULL)) {
switch (pDeviceCfg->deviceType) {
case U_DEVICE_TYPE_CELL:
errorCode = uDevicePrivateCellAdd(pDeviceCfg, pDeviceHandle);
errorCode = uDevicePrivateCellAdd(pDeviceCfg, &deviceHandleCandidate);
if (errorCode == 0) {
U_DEVICE_INSTANCE(*pDeviceHandle)->moduleType = pDeviceCfg->deviceCfg.cfgCell.moduleType;
U_DEVICE_INSTANCE(deviceHandleCandidate)->moduleType = pDeviceCfg->deviceCfg.cfgCell.moduleType;
}
break;
case U_DEVICE_TYPE_GNSS:
errorCode = uDevicePrivateGnssAdd(pDeviceCfg, pDeviceHandle);
errorCode = uDevicePrivateGnssAdd(pDeviceCfg, &deviceHandleCandidate);
if (errorCode == 0) {
U_DEVICE_INSTANCE(*pDeviceHandle)->moduleType = pDeviceCfg->deviceCfg.cfgGnss.moduleType;
U_DEVICE_INSTANCE(deviceHandleCandidate)->moduleType = pDeviceCfg->deviceCfg.cfgGnss.moduleType;
}
break;
case U_DEVICE_TYPE_SHORT_RANGE:
errorCode = uDevicePrivateShortRangeAdd(pDeviceCfg, pDeviceHandle);
errorCode = uDevicePrivateShortRangeAdd(pDeviceCfg, &deviceHandleCandidate);
if (errorCode == 0) {
U_DEVICE_INSTANCE(*pDeviceHandle)->moduleType = pDeviceCfg->deviceCfg.cfgSho.moduleType;
U_DEVICE_INSTANCE(deviceHandleCandidate)->moduleType = pDeviceCfg->deviceCfg.cfgSho.moduleType;
}
break;
case U_DEVICE_TYPE_SHORT_RANGE_OPEN_CPU:
errorCode = uDevicePrivateShortRangeOpenCpuAdd(pDeviceCfg, pDeviceHandle);
errorCode = uDevicePrivateShortRangeOpenCpuAdd(pDeviceCfg, &deviceHandleCandidate);
if (errorCode == 0) {
U_DEVICE_INSTANCE(*pDeviceHandle)->moduleType = pDeviceCfg->deviceCfg.cfgSho.moduleType;
U_DEVICE_INSTANCE(deviceHandleCandidate)->moduleType = pDeviceCfg->deviceCfg.cfgSho.moduleType;
}
break;
default:
Expand All @@ -299,6 +301,10 @@ int32_t uDeviceOpen(const uDeviceCfg_t *pDeviceCfg, uDeviceHandle_t *pDeviceHand
uDeviceUnlock();
}

if (errorCode == 0) {
*pDeviceHandle = deviceHandleCandidate;
}

return errorCode;
}

Expand Down

0 comments on commit 7df8ac9

Please sign in to comment.