C代碼 - 需要澄清的有效性

Question

您好我已經編寫了一個基於需求的代碼。

（field1_6）（field2_30）（field3_16）（field4_16）（field5_1）（field6_6）（field7_2）（field8_1） ..... 這是一個鬥式（8場）數據的。我們將一次收到20個桶，共計160個字段。 我需要根據預定義的條件取field3，field7 & fields8的值。 如果輸入參數是N，則從第一個桶取三個字段，如果是Y，我需要 從除第一個桶以外的任何其他桶取得三個字段。 如果argumnet是Y，那麼我需要一個接一個地掃描所有20個桶並檢查 桶的第一個字段不等於0，如果它是true，則獲取該桶的三個字段並退出。 我已經寫了代碼，它也工作得很好..但不那麼自信，它是有效的。 我恐怕有一段時間會崩潰。請在下面提示是代碼。

int CMI9_auxc_parse_balance_info(char *i_balance_info,char *i_use_balance_ind,char *o_balance,char *o_balance_change,char *o_balance_sign 
) 
{ 
    char *pch = NULL; 
    char *balance_id[MAX_BUCKETS] = {NULL}; 
    char balance_info[BALANCE_INFO_FIELD_MAX_LENTH] = {0}; 
    char *str[160] = {NULL}; 
    int i=0,j=0,b_id=0,b_ind=0,bc_ind=0,bs_ind=0,rc; 
    int total_bukets ; 
    memset(balance_info,' ',BALANCE_INFO_FIELD_MAX_LENTH); 
    memcpy(balance_info,i_balance_info,BALANCE_INFO_FIELD_MAX_LENTH); 
    //balance_info[BALANCE_INFO_FIELD_MAX_LENTH]='\0'; 
    pch = strtok (balance_info,"*"); 
    while (pch != NULL && i < 160) 
    { 
    str[i]=(char*)malloc(strlen(pch) + 1); 
    strcpy(str[i],pch); 
    pch = strtok (NULL, "*"); 
    i++; 
    } 
total_bukets = i/8 ; 
    for (j=0;str[b_id]!=NULL,j<total_bukets;j++) 
    { 
    balance_id[j]=str[b_id]; 
    b_id=b_id+8; 
    } 
    if (!memcmp(i_use_balance_ind,"Y",1)) 
    { 
    if (atoi(balance_id[0])==1) 
    { 
     memcpy(o_balance,str[2],16); 
     memcpy(o_balance_change,str[3],16); 
     memcpy(o_balance_sign,str[7],1); 
     for(i=0;i<160;i++) 
     free(str[i]); 
     return 1; 
    } 
    else 
    { 
     for(i=0;i<160;i++) 
     free(str[i]); 
     return 0; 
    } 
    } 
    else if (!memcmp(i_use_balance_ind,"N",1)) 
    { 
     for (j=1;balance_id[j]!=NULL,j<MAX_BUCKETS;j++) 
     { 
     b_ind=(j*8)+2; 
     bc_ind=(j*8)+3; 
     bs_ind=(j*8)+7; 
     if (atoi(balance_id[j])!=1 && atoi(str[bc_ind])!=0) 
     { 
     memcpy(o_balance,str[b_ind],16); 
     memcpy(o_balance_change,str[bc_ind],16); 
     memcpy(o_balance_sign,str[bs_ind],1); 
     for(i=0;i<160;i++) 
     free(str[i]); 
     return 1; 
     } 
     } 
    for(i=0;i<160;i++) 
    free(str[i]); 
    return 0; 
    } 
for(i=0;i<160;i++) 
free(str[i]); 
return 0; 
} 

Answer 1

我做了一個艱難的時間閱讀你的代碼，但FWIW我已經添加了一些意見，HTH：

// do shorter functions, long functions are harder to follow and make errors harder to spot 
// document all your variables, at the very least your function parameters 
// also what the function is suppose to do and what it expects as input 
int CMI9_auxc_parse_balance_info 
(
    char *i_balance_info, 
    char *i_use_balance_ind, 
    char *o_balance, 
    char *o_balance_change, 
    char *o_balance_sign 
) 
{ 
    char *balance_id[MAX_BUCKETS] = {NULL}; 
    char balance_info[BALANCE_INFO_FIELD_MAX_LENTH] = {0}; 
    char *str[160] = {NULL}; 
    int i=0,j=0,b_id=0,b_ind=0,bc_ind=0,bs_ind=0,rc; 
    int total_bukets=0; // good practice to initialize all variables 

    // 
    // check for null pointers in your arguments, and do sanity checks for any 
    // calculations 
    // also move variable declarations to just before they are needed 
    // 

    memset(balance_info,' ',BALANCE_INFO_FIELD_MAX_LENTH); 
    memcpy(balance_info,i_balance_info,BALANCE_INFO_FIELD_MAX_LENTH); 
    //balance_info[BALANCE_INFO_FIELD_MAX_LENTH]='\0'; // should be BALANCE_INFO_FIELD_MAX_LENTH-1 

    char *pch = strtok (balance_info,"*"); // this will potentially crash since no ending \0 

    while (pch != NULL && i < 160) 
    { 
    str[i]=(char*)malloc(strlen(pch) + 1); 
    strcpy(str[i],pch); 
    pch = strtok (NULL, "*"); 
    i++; 
    } 
    total_bukets = i/8 ; 
    // you have declared char*str[160] check if enough b_id < 160 
    // asserts are helpful if nothing else assert(b_id < 160); 
    for (j=0;str[b_id]!=NULL,j<total_bukets;j++) 
    { 
    balance_id[j]=str[b_id]; 
    b_id=b_id+8; 
    } 
    // don't use memcmp, if ('y'==i_use_balance_ind[0]) is better 
    if (!memcmp(i_use_balance_ind,"Y",1)) 
    { 
    // atoi needs balance_id str to end with \0 has it? 
    if (atoi(balance_id[0])==1) 
    { 
     // length assumptions and memcpy when its only one byte 
     memcpy(o_balance,str[2],16); 
     memcpy(o_balance_change,str[3],16); 
     memcpy(o_balance_sign,str[7],1); 
     for(i=0;i<160;i++) 
     free(str[i]); 
     return 1; 
    } 
    else 
    { 
     for(i=0;i<160;i++) 
     free(str[i]); 
     return 0; 
    } 
    } 
    // if ('N'==i_use_balance_ind[0]) 
    else if (!memcmp(i_use_balance_ind,"N",1)) 
    { 
    // here I get a headache, this looks just at first glance risky. 
    for (j=1;balance_id[j]!=NULL,j<MAX_BUCKETS;j++) 
    { 
     b_ind=(j*8)+2; 
     bc_ind=(j*8)+3; 
     bs_ind=(j*8)+7; 
     if (atoi(balance_id[j])!=1 && atoi(str[bc_ind])!=0) 
     { 
     // length assumptions and memcpy when its only one byte 
     // here u assume strlen(str[b_ind])>15 including \0 
     memcpy(o_balance,str[b_ind],16); 
     // here u assume strlen(str[bc_ind])>15 including \0 
     memcpy(o_balance_change,str[bc_ind],16); 
     // here, besides length assumption you could use a simple assignment 
     // since its one byte 
     memcpy(o_balance_sign,str[bs_ind],1); 
     // a common practice is to set pointers that are freed to NULL. 
     // maybe not necessary here since u return 
     for(i=0;i<160;i++) 
      free(str[i]); 
     return 1; 
     } 
    } 
    // suggestion do one function that frees your pointers to avoid dupl 
    for(i=0;i<160;i++) 
     free(str[i]); 
    return 0; 
    } 
    for(i=0;i<160;i++) 
    free(str[i]); 
    return 0; 
} 

，當你要訪問數組中的偏移一個有用的方法是創建一個映射的一個結構內存佈局。然後你將指針指向結構體的指針，並使用struct成員來提取信息，而不是你的各種memcpy的

我還建議你重新考慮一下函數的參數，如果你把它們放在結構你有更好的控制，並使該函數更具可讀性，例如

int foo(input* inbalance, output* outbalance) 

（或不管它是你正在嘗試做的）

Answer 2

我的感覺是，這段代碼非常脆弱。如果給出了很好的輸入（我不打算讓桌面檢查你的東西），它可能會工作，但如果給出一些不正確的輸入，它會崩潰或燒傷或給出令人誤解的結果。

您是否測試了意外輸入？例如：

• 假設i_balance_info爲空？
• 假設i_balance_info是「」？

• 假設輸入字符串中少於8個項目，這行代碼將執行什麼操作？

  memcpy(o_balance_sign,str[7],1); 
  

• 假設，在海峽項目[3]少於16個字符長，會有什麼這行代碼呢？

  memcpy(o_balance_change,str[3],16); 
  

我的方法來編寫這樣的代碼將防止所有這些可能性。至少我會添加ASSERT（）語句，我通常會寫明確的輸入驗證並在錯誤時返回錯誤。這裏的問題在於界面似乎不允許任何可能存在錯誤輸入的可能性。