Thread: 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

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

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

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
}

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;
}