• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 241
  • Last Modified:

Help & optimizaion

Hi!!

I'm kinda new to Linux C programming.. and I made a function that will accept a line of text with tag such as %%FIRSTNAME%% and %%LASTNAME%% and many more.. and should replace them with the correct value and then return the new string to the calling function. I used regular expression to do this and some memory function (malloc, realloc).. now.. I'll show you the function and I would like to know if there is some memory leak in this. I don't free(tag) cuz it gives me a segmentation fault and I don't understand why..

Also, I would appreciate any comment to improve my function.

So Here it is:

#include        <stdlib.h>
#include        <sys/types.h>
#include        <regex.h>
#include        <stdio.h>
#include        <string.h>

#define ARG_SIZE 1000

char *SubstituteString (char *, int *);
char *processTag(const char *, int*);

int main(int argc, char *argv[]){

        char line[] = "Welcome %%FIRSTNAME%% %%LASTNAME%%! We are pleased to invite you..";
        int err = 0;
        char *my_ret;

        my_ret = SubstituteString(line, &err);
        if (err != 0)
                printf("Error!!\n\n");

        puts(my_ret);
        return 0;
}

char *SubstituteString(char *myLine, int *err)
{
        regex_t regex;                  // regex
        int reti;                       // regex
        regmatch_t pmatch[2];           // regex
        int i, j;                       // Loop
        int start = 0;                  // regex
        char *tag;                      // tag
        char *where;                    // regex
        char *tag_return;               // tag
        char *completeLine = NULL;


/* Compile regular expression */
        reti = regcomp(&regex, "%%([[:alnum:]]+)%%", REG_EXTENDED);
        if( reti ){ fprintf(stderr, "Could not compile regex\n"); exit(1); }

/* Execute regular expression */
        completeLine = (char *) malloc(1);
        bzero(completeLine, 1);
        where = myLine;
        while(regexec(&regex, myLine + start, 2, pmatch, 0) == 0)
        {
                if ((tag = (char *) malloc(pmatch[1].rm_eo - pmatch[1].rm_so) + 1) == NULL)
                {
                        *err = EOF;
                        return NULL;
                }
       
       
                for (i = start + pmatch[1].rm_so, j = 0; i < start + pmatch[1].rm_eo; i++, j++)
                        tag[j] = myLine[i];
                tag[j] = '\0';
 
                tag_return = processTag(tag, &reti);

                // Reallocate enough memory
                completeLine = (char *) realloc(completeLine, strlen(completeLine) + pmatch[0].rm_so + strlen(tag_return));
       
                // Add the line as well as the tag_return
                strncat(completeLine, where, pmatch[0].rm_so);
                strcat(completeLine, tag_return);
       
                //update the start & where
                start += pmatch[0].rm_eo;
                where = myLine + start;
        }
                       
        completeLine = (char *) realloc(completeLine, strlen(completeLine) + strlen(where) + 1);
        strcat(completeLine, where);    // strcat the rest of the line
       
        regfree(&regex);
               
        return completeLine;
               
}


char *processTag(const char *tag, int *err)
{
        // this function is to be developped..
        // Should return a meaningful value depending of the arg tag
               
        char *ret;
        ret = (char *) malloc(strlen("This remplace the tag")+1);
        strcpy(ret, "This remplace the tag");
        return ret;
               
}
0
kuist
Asked:
kuist
1 Solution
 
kuistAuthor Commented:
I guess I could have made a recursive function... but didn't know quite well how to do it correctly =/
0
 
gj62Commented:
Yes. You have leaks.

Anytime you do a malloc() (or calloc, realloc, etc), within a function, and you don't free that memory, you have a leak, unless you return a pointer to it that memory so you can free it later...

Why can't you free tag at the end of the function, before you return???

0
 
kuistAuthor Commented:
I don't know why!!

I wanted to put:

free(tag);

just below the line :
tag_return = processTag(tag, &reti);

and I wanted to put
free(tag_return);

just below the line :
strcat(completeLine, tag_return);

but when I do this, I get a Segmentation fault.

I just tried the second free (free(tag_return);) and works well.. I didn't tried it before.

but when I uncomment the free(tag);
it gives a Segmentation fault :(

I have no idea why.
0
Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

 
marcjbCommented:
The reason you get a segmentation fault is because of this line:

  if ((tag = (char *) malloc(pmatch[1].rm_eo - pmatch[1].rm_so) + 1) == NULL)

The () are not matched correctly, so that instead of adding 1 to (pmatch[1].rm_eo - pmatch[1].rm_so), you are acutally adding 1 to the address you are assigning.  Then, if you try and free the data, you overstep what was allocated, causing a segmentation fault.  The correct line would be:

  if ((tag = (char *) malloc((pmatch[1].rm_eo - pmatch[1].rm_so) + 1)) == NULL)


Hope this helps,

Marc
0
 
kuistAuthor Commented:
Worked well!!

So by freeing Tag and tag_return... assuming I'm taking care of completeLine in the calling function, there would be no leak in this function ?
0
 
cryptosidCommented:
well then give him the points...!!!!!!!
hey and try making things simple...

cheers
cryptosid
0
 
marcjbCommented:
As far as I can tell, yes.  
0
 
kuistAuthor Commented:
Thanks a lot :)
0

Featured Post

Concerto's Cloud Advisory Services

Want to avoid the missteps to gaining all the benefits of the cloud? Learn more about the different assessment options from our Cloud Advisory team.

Tackle projects and never again get stuck behind a technical roadblock.
Join Now