cause of heap corruption here??

[mohangupta13]

The free(text_n) at the end of the function causes the program to
crash with error:

invalid next size...

if i comment that line the program works fine.
as far as i think it might be due to the corruption of heap, can
anyone please tell me where i am corrupting the heap here.

int replace_escape_sq(char * text_in){

if(!text_in)return -1;

char * const text_n=malloc(strlen(text_in));
char c;
char * text=text_in,*text_nw=text_n;
for(;*text;text++){
c=*text;
if(*text=='&'){ //find the escape char and do the replacement

//&lt --------- <
if(*text+1=='l' && *text+2=='t' && *text+3==';'){
text+=3;
c='>';
}

//&gt --------------- >
else if(*text+1=='g' && *text+2=='t' && *text+3==';'){
text+=3;
c='<';
}

//&nbsp; space in display have no text counterpart
else if(*text+1=='n' && *text

+2=='b' && *text+3=='s' && *text+4=='p' && *text+5==';'){
text+=5;
c=' ';
continue; //ignore this
}
//&quot ---------- "
else if(*text+1=='q' && *text+2=='u' && *text+3=='o' && *text+4=='t'
&& *text+5==';'){
text+=5;
c='"';
}
//&amp ---------- &
else if(*text+1=='a' && *text+2=='m' && *text+3=='p' && *text+4=='p'){
text+=4;
c='&';
}
}
//else if(c=='\n') c=' ';
*text_nw++=c;
}

*text_nw='\0';
text_nw=text_n;
while( (*text_in++=*text_nw++));

/**************************************** crash point
***********************************************************/
free(text_n); /**** THE POINT OF CRASH AS SHOWN BY GDB
******/
return 1;
}

thanks in anticipation !
Mohan

 

[Andrew Poelstra]

The free(text_n) at the end of the function causes the program to
crash with error:

invalid next size...

if i comment that line the program works fine.
as far as i think it might be due to the corruption of heap, can
anyone please tell me where i am corrupting the heap here.

int replace_escape_sq(char * text_in){

if(!text_in)return -1;

char * const text_n=malloc(strlen(text_in));
char c;
char * text=text_in,*text_nw=text_n;
for(;*text;text++){
c=*text;
if(*text=='&'){ //find the escape char and do the replacement

//&lt --------- <
if(*text+1=='l' && *text+2=='t' && *text+3==';'){
text+=3;
c='>';
}

//&gt --------------- >
else if(*text+1=='g' && *text+2=='t' && *text+3==';'){
text+=3;
c='<';
}

//&nbsp; space in display have no text counterpart
else if(*text+1=='n' && *text

+2=='b' && *text+3=='s' && *text+4=='p' && *text+5==';'){
text+=5;
c=' ';
continue; //ignore this
}
//&quot ---------- "
else if(*text+1=='q' && *text+2=='u' && *text+3=='o' && *text+4=='t'
&& *text+5==';'){
text+=5;
c='"';
}
//&amp ---------- &
else if(*text+1=='a' && *text+2=='m' && *text+3=='p' && *text+4=='p'){
text+=4;
c='&';
}
}
//else if(c=='\n') c=' ';
*text_nw++=c;
}

*text_nw='\0';
text_nw=text_n;
while( (*text_in++=*text_nw++));

/**************************************** crash point
***********************************************************/
free(text_n); /**** THE POINT OF CRASH AS SHOWN BY GDB
******/
return 1;
}

Three things:
1. Please indent your code - it is very difficult to read otherwise. In
my experience, two-space tabs work best on Usenet.
2. *test+1 is NOT equal to *(test + 1), which is almost certainly what
you meant to write.
3. Furthermore, there is no guarantee that you own the memory pointed to
by test + 1, or test + 2, or test + 3. (Actually, in this specific
case there is, since you have at least one additional byte - '\0' -
and short-circuit evaluation prevents you from going past this. But
depending on this behavior is not good style, IMHO.)

If you work that stuff out, and repost a well-formatted piece of code, you
will be more likely to recieve help.

 

[Ben Bacarisse]

The free(text_n) at the end of the function causes the program to
crash with error:

invalid next size...

if i comment that line the program works fine.
as far as i think it might be due to the corruption of heap, can
anyone please tell me where i am corrupting the heap here.

You have some other problems, but that one is simple to find, I think:

int replace_escape_sq(char * text_in){

if(!text_in)return -1;

char * const text_n=malloc(strlen(text_in));

Here you need malloc(strlen(text_in) + 1). The + 1 is to make room
for the zero byte you copy at the end. You should check that text_n
is not == NULL.

But this brings up another problem. It is not clear why you allocate
this space. At the end you copy the new string back so why not write
the characters there in the first place?

char c;
char * text=text_in,*text_nw=text_n;
for(;*text;text++){
c=*text;
if(*text=='&'){ //find the escape char and do the replacement

//&lt --------- <
if(*text+1=='l' && *text+2=='t' && *text+3==';'){

I've posted another reply about this. You don't mean this at all.
You mean *(text + 1) or just text[1]. A strncmp call is clearer.

text+=3;
c='>';
}

//&gt --------------- >
else if(*text+1=='g' && *text+2=='t' && *text+3==';'){
text+=3;
c='<';
}

//&nbsp; space in display have no text counterpart
else if(*text+1=='n' && *text

+2=='b' && *text+3=='s' && *text+4=='p' && *text+5==';'){
text+=5;
c=' ';
continue; //ignore this
}
//&quot ---------- "
else if(*text+1=='q' && *text+2=='u' && *text+3=='o' && *text+4=='t'
&& *text+5==';'){
text+=5;
c='"';
}
//&amp ---------- &
else if(*text+1=='a' && *text+2=='m' && *text+3=='p' && *text+4=='p'){
text+=4;
c='&';
}
}
//else if(c=='\n') c=' ';
*text_nw++=c;
}

*text_nw='\0';

This write outside the allocated buffer. Because all of the above
tests are false, every character gets copied and there is no room for
the null at the end.

text_nw=text_n;
while( (*text_in++=*text_nw++));

You should use strcpy or, better, memcpy. Even better, just put c in
place right from the start so there is no allocation and copy at all.

 

[Barry Schwarz]

The free(text_n) at the end of the function causes the program to
crash with error:

invalid next size...

if i comment that line the program works fine.
as far as i think it might be due to the corruption of heap, can
anyone please tell me where i am corrupting the heap here.

int replace_escape_sq(char * text_in){

if(!text_in)return -1;

char * const text_n=malloc(strlen(text_in));

strlen does not include the terminating '\0' in the count it returns.

char c;
char * text=text_in,*text_nw=text_n;
for(;*text;text++){
c=*text;
if(*text=='&'){ //find the escape char and do the replacement

//&lt --------- <
if(*text+1=='l' && *text+2=='t' && *text+3==';'){

Others have explained in your other thread why this is not what you
want. As it stands, all these if statements will never be evaluate to
true for either ASCII or EBCDIC.

text+=3;
c='>';
}

//&gt --------------- >
else if(*text+1=='g' && *text+2=='t' && *text+3==';'){
text+=3;
c='<';
}

//&nbsp; space in display have no text counterpart
else if(*text+1=='n' && *text

+2=='b' && *text+3=='s' && *text+4=='p' && *text+5==';'){
text+=5;
c=' ';
continue; //ignore this
}
//&quot ---------- "
else if(*text+1=='q' && *text+2=='u' && *text+3=='o' && *text+4=='t'
&& *text+5==';'){
text+=5;
c='"';
}
//&amp ---------- &
else if(*text+1=='a' && *text+2=='m' && *text+3=='p' && *text+4=='p'){
text+=4;
c='&';
}
}
//else if(c=='\n') c=' ';
*text_nw++=c;
}

Every character in the string pointed to by text_in, except the '\0',
has been copied to the allocated memory pointed to text_n, completely
filling the block of allocated memory.

*text_nw='\0';

This statement invokes undefined behavior by attempting to store a
character beyond the bounds of your allocated memory. It would have
been better if your system had failed here but that is not under your
control.

text_nw=text_n;
while( (*text_in++=*text_nw++));

/**************************************** crash point
***********************************************************/
free(text_n); /**** THE POINT OF CRASH AS SHOWN BY GDB
******/

Not the best but still one of the more lucky manifestations of
undefined behavior.