2015-01-03 14 views
0

我正在開發一個自定義連接函數(爲我的拼貼賦值),並且我不能讓它工作,當我打印拼接結果時,我得到了奇怪的字符。爲什麼我在C中得到錯誤的自定義連接函數結果?

這是我的代碼

typedef struct { 
int len; 
char *s; 
} string_t; 
typedef string_t* string; 

void set (string* s1, char *s); 
void concat (string* s1, string s2); 

int main(void) { 

//create variables 
    string *str1; 
    string *str2; 

    set(str1,"hello "); 
    set(str2,"world!"); 
    printf("\nconcatenate str1 and str2\n"); 
    concat(str1,*str2); 

    printf("concatenation result is:\n"); 
    //the problem is here 
    printf("%p , %s",(*str1)->s,(*str1)->s); 

    printf("\n------End------\n"); 

    return EXIT_SUCCESS; 
} 
void set(string* s1, char *s){ 

    if(s1 != NULL){ 
     if(s == NULL){ 
      (*s1)->len = -1; 
     }else{ 
      (*s1)->s = s; 
      (*s1)->len = strlen(s); 
     } 
    } 
} 

void concat (string* s1, string s2){ 
    int totalLen = (*s1)->len + (*s2).len; 
    char rslt[totalLen+1]; 
    int i=0; 
    for(i=0;i<(*s1)->len;i++){ 
     rslt[i] = (*s1)->s[i]; 
    } 
    for(i=(*s1)->len;i<totalLen;i++){ 
     int j=i-(*s1)->len; 
     rslt[i] = (*s2).s[j]; 
    } 
    rslt[totalLen] = '\0'; 

    set(s1,rslt); 
    //there is no problem when printing here 
    printf("%p , %s\n",(*s1)->s,(*s1)->s); 
} 

這是結果我得到

串連str1和str2的

0023FE2C,世界,你好!

拼接的結果是:

0023FE2C,ج@@

------結束------

我使用Eclipse IDE中。

任何人都可以幫我解決這個問題嗎?

+0

你永遠不分配一些內存。你應該在'set()'中做。目前你將一個臨時數組傳遞給'set(s1,rslt);'然後你的數據會'丟失'。另請閱讀[如何調試小程序](http://ericlippert.com/2014/03/05/how-to-debug-small-programs/)。 – honk

+0

你不能修改文字字符串,比如你用'set'創建的字符串。 – usr2564301

+0

您正將一個局部變量的指針分配給您的字符串。當函數結束時,局部變量被銷燬並且使用該指針是未定義的行爲。 http://stackoverflow.com/questions/18473731/pointer-to-local-variable-outside-the-scope-of-its-declaration –

回答

1

您的concat函數返回指向本地數組rslt的指針。該數組僅在調用封閉函數期間有效。 concat退出str1->s後包含dangling pointer。使用它會導致未定義的行爲。要解決此問題

的方法之一是這個數組複製到堆上分配內存:

void set(string* s1, const char *s){ 
    free(s1->s); 
    s1->s = strdup(s); 
    s1->len = strlen(s); 
} 
+0

非常感謝,我剛剛編輯代碼abit void set(string * s1,const char * s){(* * s1) - > s); (* s1) - > s = strdup(s); (* s1) - > len = strlen(s); } 而且工作完美。 – Said

+0

你不應該這樣做。我認爲你的聲明中有太多的指針。有關示例如何刪除它們,請參閱其他答案。 –

0

修復的樣本

#include <stdio.h> 
#include <stdlib.h> 
#include <string.h> 

typedef struct { 
    int len; 
    char *s; 
} string; 
//typedef string_t string;//no need pointer type. 

void set (string *s1, const char *s); 
void concat (string *s1, const string *s2);//Unified to pointer 
string *create(const char *s); 

int main(void) { 
    string *str1 = create(NULL);//calloc(1, sizeof(*str1));//create("");//create("hello "); 
    string *str2 = create(NULL);//calloc(1, sizeof(*str2));//create(""); 

    set(str1, "hello "); 
    set(str2, "world!"); 
    printf("\nconcatenate str1 and str2\n"); 
    concat(str1, str2); 

    printf("concatenation result is:\n"); 
    printf("%s", str1->s); 

    printf("\n------End------\n"); 
    free(str1->s);free(str1); 
    free(str2->s);free(str2); 
    return EXIT_SUCCESS; 
} 

string *create(const char *s){ 
    string *str = malloc(sizeof(*str)); 
    if(str){ 
     if(s){ 
      str->len = strlen(s); 
      str->s = strdup(s);//strdup isn't standard 
/* 
      str->s = malloc(str->len + 1); 
      if(!str->s){ 
       free(str); 
       str = NULL; 
      } else { 
       memcpy(str->s, s, str->len + 1); 
      } 
*/ 
     } else { 
      str->len = -1; 
      str->s = NULL; 
     } 
    } 
    return str; 
} 

void set(string *s1, const char *s){ 
    if(s1 != NULL){ 
     free(s1->s); 
     if(s == NULL){ 
      s1->s = NULL; 
      s1->len = -1; 
     }else{ 
      s1->s = strdup(s); 
      s1->len = strlen(s); 
     } 
    } 
} 

void concat (string *s1, const string *s2){ 
    if(!s1 || !s2 || s1->len == -1 || s2->len == -1) 
     return ; 

    int totalLen = s1->len + s2->len; 
    char rslt[totalLen+1];//char *rslt = malloc(totalLen+1); 

    strcpy(rslt, s1->s);//use standard library 
    memcpy(rslt + s1->len, s2->s, s2->len + 1); 

    set(s1, rslt); 
    //free(rslt); 
} 
+0

如果'create()'在分配空間之後調用'set()',則會避免一些重複。 –

+0

創建和分配對象的兩種API可能是。 – BLUEPIXY

0
// code readability is every bit as important as the algorithm used 
// when compiling, have all warnings enabled, and then fix them 
// OP can add parameter checking and make the str1 and str1 be pointers 
// with appropriate changes to the rest of the code 
// I changed return types from void to int 
// for set() and concat() so main could free the allocated memory areas 
// the 'software contract' concept says the sub functions in the file 
// do not need to check their parameters for validity 


#include <stdio.h> 
#include <stdlib.h> 
#include <string.h> // strlen() 
// strdup() should have been prototyped by string.h, 
// but was not on my computer 
char *strdup(const char *s); 


// dont clutter the code with frivilous typedef's 
struct string_t 
{ 
    int len; 
    char * s; 
}; 

// prototypes 
int concat (struct string_t*, struct string_t*); 
int set(struct string_t*, char*); 

int main(void) 
{ 
    //create variables 
    // your learning C, so keep it simple 
    struct string_t str1 = {0,NULL}; // good programming practice to initialize local variables 
    struct string_t str2 = {0,NULL}; 

    if(!set(&str1, "hello ")) 
    { 
     if(!set(&str2, "world!")) 
     { 
      printf("\nconcatenate str1 and str2\n"); 
      if(!concat(&str1,&str2)) 
      { 
       printf("concatenation result is:\n"); 
       printf("%p , %s",(void*)&(str1.s), str1.s); 

       printf("\n------End------\n"); 
      } // end if 
     } // end if 
    } // end if 

    free(str1.s); 
    free(str2.s); 
    return EXIT_SUCCESS; 
} // end function: main 


// <-- pString1 must point to an instance of struct string_t 
// do not free() the pNewString 
// as it is a pointer to a literal, 
// not a pointer to allocated memory 
int set(struct string_t* pString1, char* pNewString) // <-- use meaningful/descriptive names 
{ 
    int returnValue = 0; // indicate success 


    char * temp = strdup(pNewString); 
    if(NULL == temp) 
    { // then strdup failed 
     perror("strdup failed"); 
     returnValue = 1; // indicate failure 
    } 

    else 
    { // else, strdup successful 
     pString1->s = temp; 
     pString1->len = strlen(pString1->s)+1; 
    } 
    return(returnValue); 
} // end function: set 


int concat (struct string_t* pString1, struct string_t* pString2) 
{ 
    int returnValue = 0; // indicate success 

    int totalLen = pString1->len + pString2->len + 1; 
    //printf("\nbefore: string1->len =%i,string2->len=%d, totalLength=%i\n", 
    //   pString1->len, 
    //   pString2->len, 
    //   totalLen); 
    //printf("\nbefore: string1:%s, string2:%s\n", 
    //   pString1->s, pString2->s); 

    // <-- there is no room in string1->s for any more chars so: 
    char * temp; 
    if(NULL == (temp = realloc(pString1->s, totalLen))) 
    { // then realloc failed 
     perror("realloc failed"); 
     returnValue = 1; // indicate failure 
    } 

    else 
    { 
     free(pString1->s); 
     pString1->s = temp; 
     //printf("\n after realloc: str1.len:%i, strl.s:%s\n", 
     //  pString1->len, pString1->s); 

     int i=0; 
     for(;i<totalLen;i++) 
     { 
      pString1->s[strlen(pString1->s)] = pString2->s[i]; 
      pString1->s[strlen(pString1->s)] = '\0'; 
     } // end for 

     pString1->len = totalLen; 
     pString1->s[totalLen] = '\0'; 

     //printf("after: str1addr:%p , str1:%s\n",pString1->s,pString1->s); 
     //printf("\nstring1->len =%i,string2->len=%d, totalLength=%i\n", 
     //  pString1->len, 
     //  pString2->len, 
     //  totalLen); 
    } // end if 

    return(returnValue); 
} // end function: concat 
相關問題