Thread: my good friend Mr Seg Fault

  1. #1
    Registered User
    Join Date
    Jul 2012
    Location
    Australia
    Posts
    242

    my good friend Mr Seg Fault

    Hi all.

    I am trying to store words beginning with "brown" into an array of structures, from the string: "Do cows have "brown" legs? How now "brownone" cow. A cow has four "brownthree" legs." I want to exclude words that only consist of "brown". I want string1 to be reduced with each search for "brown", and then a new search is done on the new reduced string.

    So after the first "brown"(which I don't want stored), the new string to search for the next instance of "brown" is: "into an array of structures, from the string: Do cows have "brown" legs? How now "brownone" cow. A cow has four "brownthree" legs." And so on.

    So I expect the output(when I print out the array of structures) to be:
    Code:
    brownone
    brownthree
    The debugger is giving a seg fault at line 35: strcpy(string1, temp_string), which leads me to believe that temp_string is larger than the memory allocated to string1. But even if I increase the memory allocated to string1 by an extra 100 chars(which is larger than the entire original string), still a seg fault. This is my first time using realloc(in this case to decrease memory), so did I make a mistake somewhere?

    Thanks in advance.

    Code:
    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>
    
    struct strings {
        char *string1;
        int x;
    };
    
    int main(void)
    {
        struct strings data[10];
        char * temp_string;
        char * string1 = "Do cows have \"brown\" legs? How now \"brownone\" cow. A cow has four \"brownthree\" legs.";
        char * search_string = "brown";
        int loc; // location of matching word in string
        char * word; // pointer to matching word
        int len_string, len_search_string, ch = 0, array_ctr = 0, buffer_ctr = 0;
        size_t len_temp_string;
    
        puts(string1);
        len_string = strlen(string1);
        len_search_string = strlen(search_string);
        printf("len_search-string = %d\n", len_search_string);
        while((word = strstr(string1, search_string)) != NULL)
        {
            loc = word - string1;
            printf("loc = %d", loc);
            if((ch = string1[loc + len_search_string]) == '"') // to move to next char after end of search_string
            {
                len_temp_string =  strlen(&string1[loc + len_search_string + 1]); // + 1 is to move past the "
                temp_string = malloc(len_temp_string * sizeof(char));
                strcpy(temp_string, &string1[loc + len_search_string + 1]); // +1 is to move past the "
                string1 = realloc(string1, len_temp_string); // reallocate string1 to the length of the temp string
                strcpy(string1, temp_string);
                puts(string1);
                continue;
            }
            else
            {
                while((ch = string1[loc]) != '"');
                {
                    data[array_ctr].string1 = malloc(15 * sizeof(char));
                    data[array_ctr].string1[buffer_ctr] = ch;
                    loc++;
                    buffer_ctr++;
                }
                loc++;
                data[array_ctr].string1[buffer_ctr] = '\0';
                array_ctr++; // increment array to accept next word
                buffer_ctr = 0; // reset
                len_temp_string = strlen(&string1[loc]);
                strcpy(temp_string, &string1[loc]);
                string1 = realloc(string1, len_temp_string);
                strcpy(string1, temp_string);
                continue;
            }
        }
        return 0;
    }
    IDE: Code::Blocks | Compiler Suite for Windows: TDM-GCC (MingW, gdb)

  2. #2
    TEIAM - problem solved
    Join Date
    Apr 2012
    Location
    Melbourne Australia
    Posts
    1,907
    You can't realloc string1
    Fact - Beethoven wrote his first symphony in C

  3. #3
    Registered User
    Join Date
    Jul 2012
    Location
    Australia
    Posts
    242
    Oops, failed to see that one! Thanks.
    IDE: Code::Blocks | Compiler Suite for Windows: TDM-GCC (MingW, gdb)

  4. #4
    Registered User
    Join Date
    Jul 2012
    Location
    Australia
    Posts
    242
    Well, I've created string2, allocated memory to it, then copied string1 to it.

    But the debugger seems to be stuck at line 37(reallocating string 2). I can press "step into" about 5 or 6 times, and then a seg fault occurs.

    Code:
    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>
    
    struct strings {
        char *string1;
        int x;
    };
    
    int main(void)
    {
        struct strings data[10];
        char * temp_string;
        char * string1 = "Do cows have \"brown\" legs? How now \"brownone\" cow. A cow has four \"brownthree\" legs.";
        char * string2;
        char * search_string = "brown";
        int loc; // location of matching word in string
        char * word; // pointer to matching word
        int len_string1, len_search_string, ch = 0, array_ctr = 0, buffer_ctr = 0;
        size_t len_temp_string;
    
        puts(string1);
        len_string1 = strlen(string1);
        string2 = malloc(len_string1 * sizeof(char));
        strcpy(string2, string1);
        len_search_string = strlen(search_string);
        printf("len_search-string = %d\n", len_search_string);
        while((word = strstr(string2, search_string)) != NULL)
        {
            loc = word - string2;
            printf("loc = %d", loc);
            if((ch = string1[loc + len_search_string]) == '"') // to move to next char after end of search_string
            {
                len_temp_string =  strlen(&string2[loc + len_search_string + 1]); // + 1 is to move past the "
                temp_string = malloc(len_temp_string * sizeof(char));
                strcpy(temp_string, &string2[loc + len_search_string + 1]); // +1 is to move past the "
                string2 = realloc(string2, len_temp_string); // reallocate string2 to the length of the temp string
                strcpy(string2, temp_string);
                puts(string2);
                continue;
            }
            else
            {
                while((ch = string1[loc]) != '"');
                {
                    data[array_ctr].string1 = malloc(15 * sizeof(char));
                    data[array_ctr].string1[buffer_ctr] = ch;
                    loc++;
                    buffer_ctr++;
                }
                loc++;
                data[array_ctr].string1[buffer_ctr] = '\0';
                array_ctr++; // increment array to accept next word
                buffer_ctr = 0; // reset
                len_temp_string = strlen(&string1[loc]);
                strcpy(temp_string, &string1[loc]);
                string1 = realloc(string1, len_temp_string);
                strcpy(string1, temp_string);
                continue;
            }
        }
        return 0;
    }
    IDE: Code::Blocks | Compiler Suite for Windows: TDM-GCC (MingW, gdb)

  5. #5
    TEIAM - problem solved
    Join Date
    Apr 2012
    Location
    Melbourne Australia
    Posts
    1,907
    Code:
    string2 = malloc((len_string1 * sizeof(char)) + 1); // for '\0'
    Fact - Beethoven wrote his first symphony in C

  6. #6
    TEIAM - problem solved
    Join Date
    Apr 2012
    Location
    Melbourne Australia
    Posts
    1,907
    Seg fault means that you are trying to access memory which you don't have permission to.

    I'd check all your mallocs to see if any of them are failing before trying to use them
    Fact - Beethoven wrote his first symphony in C

  7. #7
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    Code:
        len_string1 = strlen(string1);
        string2 = malloc(len_string1 * sizeof(char));
        strcpy(string2, string1);
    strcpy(string2, string1) will copy strlen(string1)+1 characters to string2. Since string2 only has strlen(string1) characters allocated, the result is undefined (falling off the end of allocated memory).

    I'm guessing - but haven't bothered to check - that you've made similar mistakes with other memory allocations.

    Also, you should really check return values from malloc() and realloc() to see if they succeed.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  8. #8
    Registered User
    Join Date
    Jul 2012
    Location
    Australia
    Posts
    242
    Well, the program is almost fully working. But I am perplexed as to why the reallocation of string2 near the bottom of the code results in "Error: Not enough space"? Then the strcpy() causes the program to crash. I am decreasing the memory, so why would there not be enough space? Thank you.

    Code:
    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>
    
    struct strings {
        char *string1;
        int x;
    };
    
    int main(void)
    {
        struct strings data[10];
        char * temp_string;
        char * string1 = "Do cows have \"brown\" legs? How now \"brownone\" cow. A cow has four \"brownthree\" legs.";
        char * string2;
        char * search_string = "brown";
        int loc; // location of matching word in string
        char * word; // pointer to matching word
        int len_string1, len_search_string, ch = 0, array_ctr = 0, buffer_ctr = 0;
        size_t len_temp_string;
    
        puts(string1);
        len_string1 = strlen(string1);
        if((string2 = malloc(len_string1 * sizeof(char) + 1)) == NULL)
            perror("Error");
        strcpy(string2, string1);
        len_search_string = strlen(search_string);
        printf("len_search-string = %d\n", len_search_string);
        while((word = strstr(string2, search_string)) != NULL)
        {
            loc = word - string2;
            printf("loc = %d\n", loc);
            if((ch = string1[loc + len_search_string]) == '"') // to move to next char after end of search_string
            {
                len_temp_string =  strlen(&string2[loc + len_search_string + 1]); // + 1 is to move past the "
                if((temp_string = malloc(len_temp_string * sizeof(char) + 1)) == NULL)
                    perror("Error");
                strcpy(temp_string, &string2[loc + len_search_string + 1]); // +1 is to move past the "
                string2 = realloc(string2, len_temp_string); // reallocate string1 to the length of the temp string
                strcpy(string2, temp_string);
                puts(string2);
                continue;
            }
            else
            {
                if((data[array_ctr].string1 = malloc(15 * sizeof(char))) == NULL)
                        perror("Error");
                while((ch = string2[loc]) != '"')
                {
                    data[array_ctr].string1[buffer_ctr] = ch;
                    loc++;
                    buffer_ctr++;
                }
                loc++;
                data[array_ctr].string1[buffer_ctr] = '\0';
                array_ctr++; // increment array to accept next word
                buffer_ctr = 0; // reset
                len_temp_string = strlen(&string2[loc]);
                if((temp_string = realloc(temp_string, len_temp_string)) == NULL)
                    perror("Error");
                strcpy(temp_string, &string2[loc]);
                if((string2 = realloc(string2, len_temp_string)) == NULL)
                    perror("Error");
                strcpy(string2, temp_string);
                continue;
            }
        }
        return 0;
    }
    IDE: Code::Blocks | Compiler Suite for Windows: TDM-GCC (MingW, gdb)

  9. #9
    Registered User
    Join Date
    May 2012
    Location
    Arizona, USA
    Posts
    945
    You're still not allocating enough space to temp_string and string2 when you call realloc.

  10. #10
    SAMARAS std10093's Avatar
    Join Date
    Jan 2011
    Location
    Nice, France
    Posts
    2,694
    I would also like to point out that before entering the loop, memory is allocated for string2.Then string1 is copied into string2.
    Then the loop will get into the if body and realloc string2 into something less...What will happen with other memory?I couldn't really understand from the documentation...Maybe you should free the memory and then realloc,because we are going to use strcpy after the realloc

    The answer to my question : realloc
    Last edited by std10093; 11-16-2012 at 12:15 PM.

  11. #11
    Registered User
    Join Date
    Jul 2012
    Location
    Australia
    Posts
    242
    Well, the code does what is expected, but outputting the array of structs presents a problem. For some unknown reason, the debugger completely skips the for() loop at the end, and the program ends. Why? Is the condition of the for() loop somehow evaluating as false?

    I really didn't need an array of structs for this at all, just an array of pointers and a counter variable.

    Code:
    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>
    
    struct strings {
        char *string1;
        int x;
    };
    
    int main(void)
    {
        struct strings data[10];
        char * temp_string;
        char * string1 = "Do cows have \"brown\" legs? How now \"brownone\" cow. A cow has four \"brownthree\" legs.";
        char * string2;
        char * search_string = "brown";
        int loc; // location of matching word in string
        char * word; // pointer to matching word
        int ctr, len_string1, len_search_string, ch = 0, array_ctr = 0, buffer_ctr = 0;
        size_t len_temp_string;
    
        for(ctr = 0; ctr < 10; ctr++)
            data[ctr].x = 0;
        len_string1 = strlen(string1);
        if((string2 = malloc(len_string1 * sizeof(char) + 1)) == NULL)
            perror("Error");
        strcpy(string2, string1);
        len_search_string = strlen(search_string);
        while((word = strstr(string2, search_string)) != NULL)
        {
            loc = word - string2;
            if((ch = string1[loc + len_search_string]) == '"') // to move to next char after end of search_string
            {
                len_temp_string =  strlen(&string2[loc + len_search_string + 1]); // + 1 is to move past the "
                if((temp_string = malloc(len_temp_string * sizeof(char) + 1)) == NULL)
                    perror("Error");
                strcpy(temp_string, &string2[loc + len_search_string + 1]); // +1 is to move past the "
                if((string2 = realloc(string2, len_temp_string + 1)) == NULL) // reallocate string2 to the length of the temp string
                    perror("Error");
                strcpy(string2, temp_string);
                continue;
            }
            else
            {
                if((data[array_ctr].string1 = malloc(15 * sizeof(char))) == NULL)
                        perror("Error");
                while((ch = string2[loc]) != '"')
                {
                    data[array_ctr].string1[buffer_ctr] = ch;
                    loc++;
                    buffer_ctr++;
                }
                loc++;
                data[array_ctr].string1[buffer_ctr] = '\0';
                data[0].x++;
                array_ctr++; // increment array to accept next word
                buffer_ctr = 0; // reset
                len_temp_string = strlen(&string2[loc]);
                if((temp_string = realloc(temp_string, len_temp_string + 1)) == NULL)
                    perror("Error");
                strcpy(temp_string, &string2[loc]);
                if((string2 = realloc(string2, len_temp_string + 1)) == NULL)
                    perror("Error");
                strcpy(string2, temp_string);
                continue;
            }
        }
        free(temp_string);
        free(string2);
        for(array_ctr = 0; ctr < data[0].x; array_ctr++)
            printf("%s\n", data[array_ctr].string1);
        return 0;
    }
    IDE: Code::Blocks | Compiler Suite for Windows: TDM-GCC (MingW, gdb)

  12. #12
    SAMARAS std10093's Avatar
    Join Date
    Jan 2011
    Location
    Nice, France
    Posts
    2,694
    You are checking the wrong value i would say
    Code:
    for(array_ctr = 0; ctr < data[0].x; array_ctr++)
            printf("%s\n", data[array_ctr].string1);
    This should be array_ctr maybe... Try this out and let us know

  13. #13
    Registered User
    Join Date
    Jul 2012
    Location
    Australia
    Posts
    242
    Quote Originally Posted by std10093 View Post
    You are checking the wrong value i would say


    Code:
    for(array_ctr = 0; ctr < data[0].x; array_ctr++)
            printf("%s\n", data[array_ctr].string1);
    This should be array_ctr maybe... Try this out and let us know
    Thanks. Works now. If anyone can suggest better code for achieving the same thing, I'd really like to hear it. Seems like my program has a lot of code, for a rather simple result.

    Now I can apply this method to my main program below, but that is for another thread.
    Code:
    #include <curl/curl.h>
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    #define URL_LENGTH 100
    
    struct MemoryStruct {
        char *memory;
        size_t size;
    };
    
    struct LinkStruct {
        char *link;
        int x;
    };
    
    static size_t WriteMemoryCallback(void *contents, size_t size, size_t nmemb, void *userp);
    struct LinkStruct * GetLinks(struct MemoryStruct);
    
    int main(void)
    {
        curl_global_init(CURL_GLOBAL_ALL);
    
        CURL *myhandle;
        CURLcode result;
        struct MemoryStruct datastruct;
    
        datastruct.memory = malloc(1); // will be grown as needed by realloc
        datastruct.size = 0; // no data at this point
        if((myhandle = curl_easy_init()) == NULL)
        {
            perror("curl_easy-init error");
            exit(EXIT_FAILURE);
        }
        if((result = curl_easy_setopt(myhandle, CURLOPT_URL, "http://cboard.cprogramming.com/c-programming/")) != CURLE_OK)
        {
            perror("CURLOP_URL error");
            exit(EXIT_FAILURE);
        }
        if((result = curl_easy_setopt(myhandle, CURLOPT_WRITEFUNCTION, WriteMemoryCallback)) != CURLE_OK)
        {
            perror("CURLOPT_WRITEFUNCTION error");
            exit(EXIT_FAILURE);
        }
        if((result =  curl_easy_setopt(myhandle, CURLOPT_WRITEDATA, &datastruct)) != CURLE_OK)
        {
            perror("CURLOPT_WRITEDATA error");
            exit(EXIT_FAILURE);
        }
        if((result = curl_easy_perform(myhandle)) != 0)
        {
            perror("curl_easy_perform error");
            exit(EXIT_FAILURE);
        }
        curl_easy_cleanup(myhandle);
        GetLinks(datastruct);
    
        return 0;
    }
    
    static size_t WriteMemoryCallback(void *contents, size_t size, size_t nmemb, void *userp)
    {
        size_t realsize = size * nmemb;
        struct MemoryStruct *mem = (struct MemoryStruct *)userp;
    
        mem->memory = realloc(mem->memory, mem->size + realsize + 1);
        if (mem->memory == NULL) /* out of memory! */
        {
            printf("not enough memory (realloc returned NULL)\n");
            exit(EXIT_FAILURE);
        }
        memcpy(&(mem->memory[mem->size]), contents, realsize);
        mem->size += realsize;
        mem->memory[mem->size] = 0;
    
        return realsize;
    }
    
    struct LinkStruct * GetLinks(struct MemoryStruct datastruct)
    {
        char *new_string;
        struct LinkStruct links[50];
        char *search_string = "http://cboard.cprogramming.com/c-programming/";
        int ch; // test character
        int ctr = 0, url_count = 0;
        int location; // location of matching strings
        int dblqut; // location of "
    
        dblqut = strlen(search_string) + 1;
        while((ch = datastruct.memory[ctr]) != '\0')
        {
            location = strstr(datastruct.memory, search_string) - datastruct.memory;
            if((ch = datastruct.memory[location + dblqut]) == '"')
            {
    
            }
    
        }
    }
    IDE: Code::Blocks | Compiler Suite for Windows: TDM-GCC (MingW, gdb)

  14. #14
    SAMARAS std10093's Avatar
    Join Date
    Jan 2011
    Location
    Nice, France
    Posts
    2,694
    Ok here is my approach
    Code:
    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>
    #include <ctype.h>
    
    void initstrFound(char* array , int n ,char* searchStr)
    {
        int i;
        for( i = 0 ; i < n ; i++ )
            array[i] = 0;
        strcpy(array , searchStr);
    }
    
    int main(void)
    {
        char* answer[10];
        int answerCounter = 0;
        int remainer,i;
        char* string1 = "Do cows have \"brown\" legs? How now \"brownone\" cow. A cow has four \"brownthree\" legs$
        char* pivot = string1;
        char* searchStr = "brown";
        char strFound[20];
    
        /* We cache the length of the string to search      */
        /* because it would be pointless to call strlen     */
        /* more than once for the same string, since the    */
        /* return value of strlen will always be the same.  */
        /* And yes , without counting the null terminator.  */
    
        int searchStrLen = strlen("brown");
    
    
        /* Find occurence of string we are searching for            */
        while( (pivot = strstr(pivot , searchStr)) != NULL )
        {
            initstrFound(strFound , 20 , searchStr);
    
            /* Bypass the string we are searching for               */
            pivot += searchStrLen;
    
            /* Is the first element that pivot is pointing to       */
            /* an alphabetical letter?                              */
    
            if( isalpha(pivot[0]) )
            {
                    /* Yes it is.So save this word                  */
                    /* Which word?The word that consists of the     */
                    /* search string plus the letters that pivot is */
                    /* pointing to until a non alphabetical one     */
                    remainer = 0;
                    i = 1;
                    while( isalpha(pivot[i]) )
                            i++;
                    strncat( strFound , pivot , i );
    
                    /* strlen will return the size of the string    */
                    /* if the array is not full                     */
                    answer[answerCounter] = malloc( ( strlen(strFound) + 1 ) * sizeof(char) );
                    if( answer[answerCounter] == NULL )
                    {
                                 fprintf(stderr,"Sorry , can not allocate memory.\n\nExiting\n\n");
                            return -1;
                    }
                    strcpy( answer[answerCounter++] , strFound );
    
                    pivot += i;
    
                    /* Now we have assumed that all the matchings   */
                    /* found are less than 20 in length.We can      */
                    /* biggers strings by setting this body of if   */
                    /* inside a loop                                */
            }
        }
        for(i = 0 ; i < answerCounter ; i++)
            printf("%s\n",answer[i]);
    
        return 0;
    }
    Notice that i did not use realloc.When you think of an approach that uses realloc, think twice.If not used smartly enough, it can be a very heavy operation.

    Hope my comments help
    Last edited by std10093; 11-16-2012 at 10:54 PM.

  15. #15
    SAMARAS std10093's Avatar
    Join Date
    Jan 2011
    Location
    Nice, France
    Posts
    2,694
    For a goodnight post let me argue with myself for this comment
    Code:
    /* This is not necessairy.Can you see why?      */
                    pivot += i;
    Actually it is necessairy.

    I will prove myself wrong with a contra-example .
    Assume we remove this line of code.

    I was misleaded by the fact that if we have browntree,then strstr will move the pivot (that is now set to tree...) to the next occurrence (if any) of word brown.So it will work if we remove the line.

    Another case was to have brownbrown, which by the description of cfanatic is not exlcuded.So the pivot( which is not set to the second brown) will not be moved.But again it will work if we remove the line.Why?Because brownbrown will considered to be a match, because pivot is set to an alphabetic letter and then,when strstr won't move the pointer from the second brown, the algorithm will treat it like a single brown, like it was a separate word, thus it will exclude it.

    But , what if we had brownbrowntree ? Now the algorithm will set the pivot after the first brown and will store the whole word (brownbrowntree) .Now pivot points to something that looks like a single word --> browntree and as a result won't exclude it as did in the previous case. Instead it will go and store browntree as a different match, a new one , which is wrong.Thus the increment by i of the pivot is mandatory.

    I will edit the post with the code ( what i will do is just to remove the comment which lies above this line of code).

    Goodnight

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 2
    Last Post: 05-07-2007, 06:49 PM
  2. for a friend
    By Ajsan in forum C++ Programming
    Replies: 4
    Last Post: 02-25-2004, 11:04 AM
  3. friend function and friend classes...usage question??
    By actionbasti in forum C++ Programming
    Replies: 2
    Last Post: 10-30-2003, 10:53 PM
  4. friend
    By laasunde in forum C++ Programming
    Replies: 3
    Last Post: 11-30-2002, 10:15 AM