problems with pointer and segfault in a very basic prog

This is a discussion on problems with pointer and segfault in a very basic prog within the C Programming forums, part of the General Programming Boards category; Hi, first post here I'm writing a program in C on Linux (Ubuntu) for a school project, one of the ...

  1. #1
    Registered User
    Join Date
    Dec 2008
    Posts
    15

    problems with pointer and segfault in a very basic prog

    Hi, first post here
    I'm writing a program in C on Linux (Ubuntu) for a school project, one of the modules of the program is getting two arguments (patt and repl) and is also taking lines from stdin (using getline()) and must replace every occurrence of patt with repl.
    Two problems:
    A. I get segmentation faults when I'm trying to free the pointer allocated by getline()
    B. Even if I comment out that free() statement, the string I get after running the replace function is printed as if no replacement occured! I'm passing a pointer to that string to the replacing function.
    here's the short version of the code:
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <unistd.h>
    #include <string.h>
    
    int strreplace(char *str, char *patt, char *repl);
    
    int main(int argc, char** argv) {
        char *str;             //replace pattern
        size_t rsize;           //num of read files by getline()
        int nbytes;
        
        char *patt = argv[1];   //pattern to look for
        char *repl = argv[2];   //replace found pattern with this
        
        str = NULL;     //getline() needs NULL and 0 in order to allocate new mem
        rsize = 0;
    
        //getline() allocates the need memory for the line in to *str
        //it is user's responsibility to free() the allocated mem
        if((nbytes = getline(&str, &rsize, stdin)) < 0) {   //end of file
            exit(1);
        }
        
        if( argc == 3 && (strstr(str, patt) != NULL) ) {
            strreplace(str, patt, repl);        //this function replaces patt with repl
            printf("str: %s", str);
        }
        
        return (EXIT_SUCCESS);
    }
    
    int strreplace(char *str, char *patt, char *repl) {
        char *occ = NULL;              //ptr to first char of first occurence of the pattern in str
        char *temp;             //temp string to hold str
        int poss, pose;         //poss - pos of the first char of patt in str, pose - after last char of patt
        int diff = (strlen(repl) - strlen(patt));
        int tempsize;   
        
        while((occ = strstr(str, patt)) != NULL) {
            tempsize = strlen(str) + diff + 1;
            temp = (char*)malloc(sizeof(char)*(tempsize));
            
            poss = strlen(str) - strlen(occ);       //getting the pos of the first char of patt in str
            pose = poss + strlen(patt);             //getting pos of the char after last char of patt in str
            
            memcpy(temp, str, poss);
            memcpy(temp + poss, repl, strlen(repl));
            memcpy(temp + poss + strlen(repl), str + pose, strlen(&(str[pose])));
            temp[tempsize] = '\0';
    
            free(str);          // here I get the seg.fault if i don't comment this out
    
            str = (char*)malloc(sizeof(char)*tempsize);
            memcpy(str, temp, tempsize);
    
            free(temp);
    
            //printf("-----\nstr: %s strlen: %d\n------\n", str, strlen(str));
        }
        
        return 0;   //success
    }
    Thanx

    ps. I've also posted this question on the Ubuntu forums (no answer yet )
    http://ubuntuforums.org/showthread.php?t=1022846

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,794
    You might want to to describe your strreplace algorithm in words and/or pseudo-code. It also looks like you intend to change the subject string in strreplace, which means that you should be passing a pointer to it (like how the GNU extension of getline() works).
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  3. #3
    Registered User
    Join Date
    Dec 2008
    Posts
    15
    yeap, I want the function to change the string I am passing to it (which SHOULD be happening, coz I'm passing a pointer). The alg goes like this:

    Code:
    while occurrence of patt found in str {
        copy the portion of the string UNTIL the first occurrence of patt in to a temp string
        copy repl to the end of the temp string
        copy the portion that comes AFTER the first occurrence of patt in to temp string
        copy temp string in to str
    }
    thanx for your help

  4. #4
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,794
    Quote Originally Posted by melight
    yeap, I want the function to change the string I am passing to it (which SHOULD be happening, coz I'm passing a pointer).
    You are passing a pointer to a character in the string, hence all you can change is the contents of the string, not the string itself. If you try to change the string itself, all you will change is your local copy of the pointer.

    Quote Originally Posted by melight
    The alg goes like this:
    At a glance, I think that it would work. However, it is probably easier to only change the subject string after the loop. This means that you only copy to the temporary string and reallocate it as needed. When the loop ends, you destroy the original subject string (i.e., by using free()) and then assign the temporary string to it (this is an assignment of pointers, not characters). Of course, this means that you must not destroy the temporary string at the end (but you should destroy it before the end of the main function).
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  5. #5
    Registered User
    Join Date
    Dec 2008
    Posts
    15
    Going to check that out now
    Last edited by melight; 12-27-2008 at 09:16 AM. Reason: hasty reader lol

  6. #6
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,794
    Quote Originally Posted by melight
    Wow!! Big surprise here. What do you mean local copy of the pointer? I was sure i'm passing by reference, especially if we're talking about a pointer.
    Ah, but you need to pass the pointer by reference, since you are going to change the string itself, not just the contents of the string.

    Quote Originally Posted by melight
    How do I make it actually change?
    Change the parameter to be a pointer to a pointer to char. You also need to change the main function such that you pass the address of str to strreplace, much like what you did when using getline.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  7. #7
    Captain Crash brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,246
    I like how you wrote an explicit comment about how you need to free() the memory returned by getline() and then completely fail to do so... I like to do that every once in a while to give everyone I work with a huge headache.
    Code:
    //try
    //{
    	if (a) do { f( b); } while(1);
    	else   do { f(!b); } while(1);
    //}

  8. #8
    Registered User
    Join Date
    Dec 2008
    Posts
    15
    Quote Originally Posted by brewbuck
    I like how you wrote an explicit comment about how you need to free() the memory returned by getline() and then completely fail to do so... I like to do that every once in a while to give everyone I work with a huge headache.
    Note taken, doesn't really fix my problem though (I've cut down the actual code where I do free() to something readable and relevant) .

    I've tried both things you've suggested laserlight, but still a no go, i keep getting illegal free errors and the like from all over probably coz my implementation sux. Is there a good/known way to do such things (replace a pattern in a string with another pattern)? I've googled it but only came up with stuff for higher languages

  9. #9
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,794
    Quote Originally Posted by melight
    I've tried both things you've suggested laserlight, but still a no go, i keep getting illegal free errors and the like from all over probably coz my implementation sux.
    What is your implementation now?
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  10. #10
    Registered User
    Join Date
    Dec 2008
    Posts
    15
    there you go:

    Code:
    #define _GNU_SOURCE
    #include <stdio.h>
    #include <stdlib.h>
    #include <unistd.h>
    #include <string.h>
    
    char* strreplace(char *str, char *patt, char *repl);
    int main(int argc, char** argv) {
        char *str;             //replace pattern
        char *somestr;
        size_t rsize;           //num of read files by getline()
        int nbytes;
        
        char *patt = argv[1];   //pattern to look for
        char *repl = argv[2];   //replace found pattern with this
        
        while(1) {
            str = NULL;     //getline() needs NULL and 0 in order to allocate new mem
            somestr = NULL;
            rsize = 0;
    
            if((nbytes = getline(&str, &rsize, stdin)) < 0) {   //end of file
                break;
            }
    
            if( argc == 3 && (strstr(str, patt) != NULL) ) {
                somestr = strreplace(str, patt, repl);        //this function replaces patt with repl
                free(str);
                printf("str: %s", somestr);
            }
    
            free(somestr);
        }
        
        return (EXIT_SUCCESS);
    }
    
    //replace all patt with repl in str
    char* strreplace(char *stri, char *patt, char *repl) {
        char *occ = NULL;              //ptr to first char of first occurence of the pattern in str
        char *temp;             //temp string to hold str
        int poss, pose;         //poss - pos of the first char of patt in str, pose - after last char of patt
        int diff = (strlen(repl) - strlen(patt));
        int tempsize;
        char* str = (char*)malloc(sizeof(char)*(strlen(stri) + 1));
        strcpy(str, stri);
        
        while((occ = strstr(str, patt)) != NULL) {
            tempsize = strlen(str) + diff + 1;
            temp = (char*)malloc(sizeof(char)*(tempsize));
            
            poss = strlen(str) - strlen(occ);       //getting the pos of the first char of patt in str
            pose = poss + strlen(patt);             //getting pos of the char after last char of patt in str
            
            memcpy(temp, str, poss);
            memcpy(temp + poss, repl, strlen(repl));
            memcpy(temp + poss + strlen(repl), str + pose, strlen(&(str[pose])));
            temp[tempsize] = '\0';
    
            free(str);
            str = (char*)malloc(sizeof(char)*tempsize);
            memcpy(str, temp, tempsize);
    
            free(temp);
        }
    
        return str;   //success
    }

  11. #11
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,794
    What are the error messages?

    Frankly, I think you are doing too much malloc()ing and copying. Since you are not doing the replacing in-place, just have one return string. Copy over from the source string into this string, and expand it with realloc() if necessary. With the use of realloc(), there is no need to free() this return string in strreplace().
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  12. #12
    Registered User
    Join Date
    Dec 2008
    Posts
    15
    Ok, thanx a lot everyone, I got it working, a function which replaces patt1 with patt2 in a given string and changes it's size if needed. It also takes care of special cases - empty second pattern, when first pattern is a substring of the second pattern and vice versa.
    I'm also pretty sure there are no memo leeks!

    Code:
    //replace all patt with repl in str
    int strreplace(char **str, char *patt, char *repl) {
        char *t;
        char *occ = NULL;                       //ptr to first char of first occurence of the pattern in str
        int diff = strlen(repl) - strlen(patt); //to calculate if we need to realloc
        int poss, pose, posne;
    
        int strsize = strlen(*str) + 1; //including '\0'
        char *temp;
    
        temp = malloc(sizeof(char)*(strsize));
        strcpy(temp, (*str));
    
        int p = 0;  //tells us where to start searching for pattern
        while((occ = strstr(&(*str)[p], patt)) != NULL) {
            poss = strlen(*str) - strlen(occ);                  //start of occurence
            pose = strlen(*str) - (strlen(occ) - strlen(patt)); //end of occurence
            posne = pose + diff;                                //end of new pattern
    
            if(diff > 0) {  //realloc()ing if more space is needed
                strsize += diff;
                if((t = realloc(*str, strsize)) == NULL) {
                    perror("filter: failed to realloc");
                    free(temp);
                    return -1;
                }
                else
                    *str = t;
    
                if((t = realloc(temp, strsize)) == NULL) {
                    perror("filter: failed to realloc");
                    free(temp);
                    return -1;
                }
                else
                    temp = t;
            }
    
            strcpy(&((*str)[poss]), repl);
            strcpy(&((*str)[posne]), &temp[pose]);
            strcpy(temp, *str);
    
            p = posne;  //start searching in new iteration from posne
        }
    
        free(temp);
        return 0;
    }

  13. #13
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,794
    Good to see that you have come up with a working solution. Here would be my attempt to practice what I preached in earlier posts in this thread:
    Code:
    int strreplace(char **subject, const char *search, const char *replacement) {
        size_t search_len = strlen(search);
        size_t replacement_len = strlen(replacement);
        char *subject_ptr = *subject;
        char *found;
        char *result = NULL;
        size_t result_size = 1; /* Account for null character. */
    
        /* Search for search string in subject. */
        while ((found = strstr(subject_ptr, search)) != NULL) {
            /* Expand result. */
            size_t passed_len = found - subject_ptr;
            size_t temp_result_size = result_size + passed_len + replacement_len;
            char *temp_result;
            if ((temp_result = realloc(result, temp_result_size)) == NULL) {
                /* Error: failed to reallocate memory. */
                free(result);
                return -1;
            }
    
            /* Copy the subject and replacement. */
            result = temp_result;
            temp_result += result_size - 1;
            result_size = temp_result_size;
            strncpy(temp_result, subject_ptr, passed_len);
            strncpy(temp_result + passed_len, replacement, replacement_len);
    
            /* Continue the search of search string in subject. */
            subject_ptr += passed_len + search_len;
        }
    
        /* Only change the subject if there is a replacement. */
        if (result != NULL) {
            /* Expand result to final size. */
            size_t passed_len = strlen(subject_ptr);
            char *temp_result;
            if ((temp_result = realloc(result, result_size + passed_len)) == NULL) {
                /* Error: failed to reallocate memory. */
                free(result);
                return -1;
            }
    
            /* Copy the rest of the subject to the result. */
            result = temp_result;
            temp_result += result_size - 1;
            strncpy(temp_result, subject_ptr, passed_len);
            temp_result[passed_len] = '\0';
    
            /* Write out the result. */
            free(*subject);
            *subject = result;
        }
    
        return 0;
    }
    One advantage of my implementation over yours is that it makes only a single pass over the subject string where copying is concerned. Your algorithm copies to the temporary before the loop and then on each iteration, in addition to actually copying the parts of the string that need copying.

    One disadvantage (which can be removed) of my implementation compared to yours is that it performs reallocations whenever a match is found, whereas you only allocate once when the replacement string is not longer than the search string. On the other hand, when the replacement string is longer than the search string, my version makes about half as many reallocations as yours.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Struct Pointer Problems. (Warning: Long Post.)
    By Phoenix940 in forum C Programming
    Replies: 1
    Last Post: 11-30-2008, 09:04 PM
  2. Pointer and segfaults question
    By kzar in forum C Programming
    Replies: 5
    Last Post: 09-15-2005, 09:03 AM
  3. Replacing chars in Pointer String results in Segfault
    By discostu in forum C Programming
    Replies: 2
    Last Post: 03-08-2003, 02:38 PM

Tags for this Thread


1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21