Cコードを明らかにする必要があるの効果
https://stackoverflow.com/questions/1542001
italiano
english
français
española
中国
日本の
العربية
Deutsch
한국어
Português
Russian質問
こんにちはさんにおすすめしたいコードに基づく要件を満たす。
(field1_6)(field2_30)(field3_16)(field4_16)(field5_1)(field6_6)(field7_2)(field8_1).....このバケット(8分野のデータです。を受けて、さらに20のバケツでは、ランジスタを接続することに全160ます。私の価値観のfield3,field7&fields8に基づく所定の条件です。場合に於入力引数がN次の三つの分野から1バケットの場合はY iが必要 の三つの分野からその他のバケット以外の1。まargumnet Yしいスキャンは20バケットの後にその他のチェック 最初のフィールドのインターネット回線のスピードするものではなく、0の場合はtrueをフェッチの分野でのバケットは、出口がございます。さんにおすすめしたいのコードとそのもの微細..でもそれだけではないので自信はeffctive.私のクラッシュします。ご提案ください以下のコードです。
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;
}
解決
私はあなたのコードが、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;
}
あなたは配列内のオフセットをアクセスしたい役立つ技術は、メモリレイアウトをマップする構造体を作成することです。その後、構造体のポインタにポインタをキャストし、代わりにあなたの様々なmemcpyの年代の情報を抽出するために、構造体のメンバを使用する
私はまた、あなたが構造体でそれらのすべてのを置く場合は、よりよい制御を持っており、例えば関数が読みやすくなり、一般的な機能にパラメータを再考お勧めします。
int foo( input* inbalance, output* outbalance )
（または何でもそれはあなたがやろうとしている場合）。
他のヒント
私の感じがこのコードは非常に脆さが発生してしまう。でも仕事が入力されていな提案をデスクチェックのものまでが変更されていない場合は一部誤入力ではクラッシュは、燃焼又は誤解を招く。
い試験で予想外の入力?例えば:
- 仮i_balance_infoはnull?
- 仮i_balance_infoは""?
いに満たないた8項目の入力文字列、このコンポーネント。
memcpy(o_balance_sign,str[7],1);この項目str[3]では以上16文字、何がこのコンポーネント。
memcpy(o_balance_change,str[3],16);
私の文章などをコードするに保護するすべてのeventualities.少なくとも思ASSERT()諸表は、普段、明示的な入力検証を返したときにエラーが発生します。ここでの問題は、界面にいないであろうということができる可能性があるかもしれない悪い入力します。
所属していません StackOverflow
