Heap Corruption When freeing memory?

[Clinthill98]

My Function...

	
	
int findpattern( char * source, char * start, char * end)
{
    char * start_ptr = strstr(source, start);
    if (start_ptr != NULL)
    {
    char * end_ptr = strstr(start_ptr, end);
    if (end_ptr != NULL)
    {

    start_ptr += strlen(start);
    end_ptr -= 1;

    printf("start_ptr = %c\n", *start_ptr);
    printf("end_ptr= %c\n", *end_ptr);
    printf("findpattern: start = %d\n", start_ptr);
    printf("findpattern: end= %d\n", end_ptr);
    printf("= %d", (end_ptr-start_ptr));

/* I must write one more byte to avoid this heap corruption.? Why is this. I included the terminating character at the end. */
//char * textinpattern = (char*) malloc((end_ptr-start_ptr)+1);

/* the fix... */

char * textinpattern = (char*) malloc((end_ptr-start_ptr)+2);

    memcpy(textinpattern, start_ptr, (end_ptr-start_ptr)+1);
    memcpy(textinpattern+(int)(end_ptr-start_ptr)+1, "\0", 1);
    
    printf("%s", textinpattern);

    free (textinpattern);
    return 1;
    }
    }else{
    return 0;
    }

} 

So I have to write one more byte of memory? I don't see why.. here is the problem code

	
	
char * textinpattern = (char*) malloc((end_ptr-start_ptr)+1); 

I included the terminating character to end the string. ? OR.. did I miss count

[Cat]

Say start_ptr was 0x10000000 and end_ptr was 0x10000005; your code is copying the string between these inclusive of both ends. End-start is 5.

You need six bytes to store the 6 positions in the string (0x1...0, 1, 2, 3, 4, 5). You need a 7th byte to store the terminating null.

The correct size to malloc is end-start+2 characters. End-start+1 is the length of the data, and then you need to allocate length+1 bytes to make room for the null termination.

You can rewrite like this to make it more clear (I also got rid of the one byte memcpy in favor of array syntax which I find easier to read). Added some comments to make it easier to follow if you don't understand:

    int dataLength = (int)(end_ptr-start_ptr)+1;

    // this allocates space for textinpattern[0]...textinpattern[dataLength]
    char * textinpattern = (char*) malloc(dataLength+1); 

    // this sets textinpattern[0]...textinpattern[dataLength-1]
    memcpy(textinpattern, start_ptr, dataLength);

    // this sets the final terminating null at the last position in the memory we allocated
    textinpattern[dataLength]='\0';

[iMalc]

These things work better when you adopt the convention that 'end' is actually one-past-the-end. It typically eliminates places where you have to add or subtract one, or in this case you'd only need to add one and not two.
Not to mention that regardless you should make it clear what convention you are using.

[Clinthill98]

ah thanks guys.. ya threw me off.. I miss counted one 1 byte in the text in the pattern. ty for the help