Embedded Design Guidelines 3: Avoiding Negative Examples

A fatal example:

/**
 * Ministry of Transport 808 session attribute acquisition event
 * @param sender: initiator
 * @param attr_type: attribute type, refer to TJtMsg808SessionCompAttrEnum
 * @param param1: parameter 1
 * @param param2: parameter 2
 * @return refer to TJtMsg808SessionCompResultEnum; >0: all succeeded, indicating length; 0 indicates return value is also successful, see specific description
 */
typedef int(*TJtMsg808SessionCompDoAttrEvent)(void *sender, int attr_type, void *param1, long param2);

Int32 get_attribute(TBsJt808FrameChannel *channel, int attr_type, void *param1, long param2)
{
    Int32 result = kJtMsg808SessionCompResultFail;

//-------------- 808 session attributes ----------------//
    switch (attr_type)
    {
    /// Province ID
    case kJtMsg808SessionCompAttrProvinceId:
    { 
        result = 33;   
    }
    break;

    /// City ID
    case kJtMsg808SessionCompAttrCityId:
    {
        result = 330100;
    }
    break;

    /// Terminal model
    case kJtMsg808SessionCompAttrTrmModel:
    {
        UInt8 trm_model[] = "PXXX-01-01";
        memcpy(param1, trm_model, sizeof(trm_model) - 1);
        result = sizeof(trm_model) - 1;    
    }
    break;

    /// Terminal ID
    case kJtMsg808SessionCompAttrTrmId:
    {
        UInt32 term_id = 0;
        if  (ParamGetDevTerminalId(&term_id) < 0)
            term_id = 2023010814;
        else
        { 
            BsJt808FrameTermIdToBcd(term_id, param1, param2);
            result = param2;
        }
    }
    break;

    /// Server count
    case kJtMsg808SessionCompAttrServerHostCount:
        result = 2;
        break;

    /// Server parameters, split host and port by colon
    case kJtMsg808SessionCompAttrServerHostParam:
    {
        UInt8 main_host[84] = { 0 };
        UInt32 port = 0;
        UInt16 len = (UInt16)param2;
        UInt16 index = param2 >> 16;     
        UInt8 normal_mode = 0;

        // If attribute acquisition fails, default to factory mode
        if (ParamGetDevTerminalMode(&normal_mode) < 0)
            normal_mode = 0;
        
        Int32 real_len = 0;
        if (ParamGetObosSvrTcpPort(&port) >= 0)
        {
            if (index == 1)
                real_len = ParamGetObosBIp(main_host, 64);
            else
                real_len = ParamGetObosMIp(main_host, 64);
        }
        if (real_len > 0)
        {
            result = BsJt808FrameStrnlen((char *)main_host, real_len);

            result += snprintf((char *)&main_host[result], sizeof(main_host) - result, ":%d", port);
            result = result > sizeof(main_host) ? sizeof(main_host) : result;
            ALOG_I("Use Test IP:PORT --> %s", main_host);
        }
        else
        {
            static char const kDefaultIpPort[] = "www.example:10005";
            strcpy((char *)main_host, kDefaultIpPort);
            result = strlen(kDefaultIpPort);
        }

        if (param1 != NULL)
        {
            result = result > len ? len : result;
            memcpy(param1, main_host, result);
        }
    }
    break;

    default: break;
    }

    return result;
}  

Optimization plan:

• Problem Analysis: Analyzing this example, the reason for its complexity is that it mixes multiple types of parameter reading functions, not only are there many parameters, but importantly, the functionality of the parameters varies greatly depending on different cases, and it lacks relevant comments.
• Root Cause: Too many parameter types
• In-depth Analysis: Although there are many parameter types, they can be divided into “integer numeric type” and “variable-length byte type”.
• Solution: Split the interface into two, one for reading integer data and one for reading variable-length data.

/**
 * @brief Query integer value method callback
 * @param  owner  User object
 * @param  id     Parameter ID
 * @param  type   Parameter type caller ensures only u8, u16, u32 three types
 * @param  is_success  Whether the query is successful
 * @return UInt32 The data content queried
 */
typedef UInt32 (*TGetAttrUInt)(void *owner, UInt32 id, TMultiPType type, Bool *is_success);

/**
 * @brief Query variable-length parameter method callback
 * @param  owner  User object
 * @param  id     Parameter ID
 * @param  buffer Query cache area
 * @param  buff_len Query cache area length
 * @return Int32 The length of the queried data content <=0: failure, >0: data length
 */
typedef Int32 (*TGetAttrBin)(void *owner, UInt32 id, UInt8 *buffer, Int32 buff_len);

• Modification Plan Analysis: By adding an interface, the function of the function becomes “purer”, the cost of adding interfaces is smaller than familiarizing with complex adaptation logic, and the probability of errors is smaller.