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라면 1 번지 이외의 다른 버킷에서 세 개의 필드를 가져와야합니다. Argumnet이 Y 인 경우 20 개의 버킷을 모두 스캔하고 버킷의 첫 번째 필드가 0이 아닌지 확인해야하며 사실이라면 해당 버킷의 세 필드를 가져 오십시오. 나는 코드를 작성했고 그 코드도 잘 작동합니다. 그러나 그것이 effcivety라고 확신하지는 않습니다. 때까지 충돌이 두렵습니다. 아래는 코드라고 제안합니다.
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);
그러한 코드를 작성하는 데 대한 나의 접근 방식은 그러한 모든 사건으로부터 보호하는 것입니다. 최소한 Assert () 문을 추가 할 것입니다. 일반적으로 명시 적 입력 유효성 검사를 작성하고 나쁜 경우 반환 오류를 작성합니다. 여기서 문제는 인터페이스가 입력이 잘못 될 수있는 가능성을 허용하지 않는 것 같습니다.
