Rajaram-Regupathy / libtypec

“libtypec” is aimed to provide a generic interface abstracting all platform complexity for user space to develop tools for efficient USB-C port management. The library can also enable development of diagnostic and debug tools to debug system issues around USB-C/USB PD topology.
34 stars 4 forks source link

argument num_pdo is being passed by value and is being updated, which looks errenous #14

Closed ColinIanKing closed 1 year ago

ColinIanKing commented 1 year ago

The function parameter num_pdo is being passed by value and not by reference. In line 948 it is being updated but this value is a pass by value argument, so it only gets updated in the function and the caller does not see the change. If the updated value for num_pdo needs to be returned to the caller it should be passed by reference, e.g.:

static int libtypec_sysfs_get_pdos_ops(int conn_num, int partner, int offset, int num_pdo, int src_snk, int type, unsigned int pdo_data) { / .. etc ... / *num_pdo = num_pdos_read; return num_pdos_read; }

Alternatively, num_pdo is not meant to be used and can be removed. This needs fixing one way or the other.

    CID 436132 (#1 of 1): Parse warning (PW.PARAM_SET_BUT_NOT_USED)
1. param_set_but_not_used: parameter "num_pdo" was set but never used
 880static int libtypec_sysfs_get_pdos_ops(int conn_num, int partner, int offset, int num_pdo, int src_snk, int type, unsigned int *pdo_data)
 881{
 882        struct stat sb;
 883        int num_pdos_read = 0;
 884        char path_str[512], port_content[512 + 256];
 885        DIR *typec_path , *port_path;
 886        struct dirent *typec_entry, *port_entry;
 887
 888        snprintf(path_str, sizeof(path_str), SYSFS_TYPEC_PATH "/port%d", conn_num);
 889
 890        if (lstat(path_str, &sb) == -1)
 891        {
 892                printf("Incorrect connector number : failed to open, %s", path_str);
 893                return -1;
 894        }
 895
 896        if (partner == 0)
 897        {
 898                if(src_snk)
 899                        snprintf(path_str, sizeof(path_str), SYSFS_TYPEC_PATH "/port%d/usb_power_delivery/source-capabilities", conn_num);
 900                else
 901                        snprintf(path_str, sizeof(path_str), SYSFS_TYPEC_PATH "/port%d/usb_power_delivery/sink-capabilities", conn_num);
 902        }
 903        else
 904        {
 905                if(src_snk)
 906                        snprintf(path_str, sizeof(path_str), SYSFS_TYPEC_PATH "/port%d-partner/usb_power_delivery/source-capabilities", conn_num);
 907                else
 908                        snprintf(path_str, sizeof(path_str), SYSFS_TYPEC_PATH "/port%d-partner/usb_power_delivery/sink-capabilities", conn_num);                
 909        }
      CID 436122: Time of check time of use (TOCTOU) [[select issue](https://scan8.scan.coverity.com/defectInstanceId=12810727&fileInstanceId=70836183&mergedDefectId=436122)]
 910        if (lstat(path_str, &sb) == -1)
 911                return -1;
 912
 913        typec_path = opendir(path_str);
 914        if (typec_path == NULL)
 915                return -1;
 916
 917        while ((typec_entry = readdir(typec_path)))
 918        {
 919                memset(port_content, 0, sizeof(port_content));
 920                snprintf(port_content, sizeof(port_content), "%s/%s", path_str,typec_entry->d_name);
 921
 922                if(strstr(typec_entry->d_name, "fixed"))
 923                {
 924                        pdo_data[num_pdos_read++] = get_fixed_supply_pdo(port_content,src_snk);
 925
 926                }
 927                else if(strstr(typec_entry->d_name, "variable"))
 928                {
 929                        pdo_data[num_pdos_read++] = get_variable_supply_pdo(port_content,src_snk);
 930
 931                }
 932                else if(strstr(typec_entry->d_name, "battery"))
 933                {
 934                        pdo_data[num_pdos_read++] = get_battery_supply_pdo(port_content,src_snk);
 935
 936                }
 937                else if(strstr(typec_entry->d_name, "programmable"))
 938                {
 939                        pdo_data[num_pdos_read++] = get_programmable_supply_pdo(port_content,src_snk);
 940
 941                }
 942                
 943                
 944        }
 945
 946        closedir(typec_path);
 947        
 948        num_pdo = num_pdos_read;
 949
 950        return num_pdos_read;
 951
 952}
Rajaram-Regupathy commented 1 year ago

thanks fixed