"Replace" function goes in overflow

[pater]

I wrote this function by myself, but program crashes when this func
gets called... why?

------

void replace(char *data, char *what, char *with) {
int i, ldata = strlen(data), lwhat = strlen(what), lwith = strlen
(with);

char *buffer = (char *) malloc(ldata);
for(i=0;i<ldata-1;i++)
if(strncmp(&data, what, strlen(what)) == 0) {
if(lwhat<lwith) buffer = realloc(buffer, ldata+lwith-lwhat);
memcpy(&buffer, with, lwith);
i += lwith-1;
} else memcpy(&buffer, &data, 1);

memcpy(&buffer[ldata], 0, 1);
data = buffer;
}

--------

Some doubts then:
1) malloc returns a pointer, no? so, the "buffer" var should contain
the address of the heap address, shouldn't it?
2) If so, memcpy wants a pointer as 1st and 2nd argument.. why should
i pass &buffer instead of simply buffer?

I have still some doubts about pointers... and reference/dereference
operators..

 

[Richard]

I wrote this function by myself, but program crashes when this func
gets called... why?

------

void replace(char *data, char *what, char *with) {
int i, ldata = strlen(data), lwhat = strlen(what), lwith = strlen
(with);

char *buffer = (char *) malloc(ldata);
for(i=0;i<ldata-1;i++)
if(strncmp(&data, what, strlen(what)) == 0) {
if(lwhat<lwith) buffer = realloc(buffer, ldata+lwith-lwhat);
memcpy(&buffer, with, lwith);
i += lwith-1;
} else memcpy(&buffer, &data, 1);

memcpy(&buffer[ldata], 0, 1);
data = buffer;
}

--------

Some doubts then:
1) malloc returns a pointer, no? so, the "buffer" var should contain
the address of the heap address, shouldn't it?
2) If so, memcpy wants a pointer as 1st and 2nd argument.. why should
i pass &buffer instead of simply buffer?

I have still some doubts about pointers... and reference/dereference
operators..

Your post could well be a well constructed troll to get the usual regs
tripping over themselves.

Get rid of that horrible one liner where you compare
lwhat & with and then realloc on a single line. Multiple statements on a
single line are not friendly and are, invariably, harder to debug.

Problems such as you have are as much about the way you think about
programming as they are about the offending line.

The first thing I note is that you realloc to make a bigger buffer but
then dont reset ldata so for sure the memcpy at the end is wrong in some
cases. The rest I would look at the locals in a debugger to discover
.....

Also: Look at your uses of strlen. Are you sure you want the
strlen and not the buffer len? Its highly probable there are issues
there. e.g strlen of "hello" is 5 but the buffer to hold it is 6
long. That type of thing. The old n-1 issues. Also, reuse what you have
: you have a lwhat but you dont use it in the strncmp line. Also, are
you sure you want memcpy? Did you look at the manual for it? Did you
possibly need memmove?

Some sage advice:

Show us that you have at least ATTEMPTED to debug it yourself using a
debugger (ignore the people here who tell you debuggers are for losers -
debuggers are there for a reason and can catch the crash and educate you
quicker than you can post your question and then filter the replies).

 

[Eric Sosman]

I wrote this function by myself, but program crashes when this func
gets called... why?

------

void replace(char *data, char *what, char *with) {
int i, ldata = strlen(data), lwhat = strlen(what), lwith = strlen
(with);

These would be better as size_t variables, but that's
probably not a big deal unless the strings are rather long.

char *buffer = (char *) malloc(ldata);

Three points: First, if you need to use a (char*) cast to
silence a compiler complaint, it means you have forgotten to
include <stdlib.h> and that silencing the complaint didn't fix
the error. Second, malloc() will return NULL if it is unable
to find enough memory for your request, and since you don't
check for this possibility your program will misbehave if it
ever happens. Third, I suspect you have forgotten to leave
room for the '\0' that marks the end of a string.

for(i=0;i<ldata-1;i++)

The "-1" looks bogus. (Thinks: Is it bogus? Yes, I'm
quite sure it's bogus. Consider data="xy", what="y", and
observe that the loop will never find the "y" in data.)

if(strncmp(&data, what, strlen(what)) == 0) {

Since you've already computed lwhat, why not use it here?

if(lwhat<lwith) buffer = realloc(buffer, ldata+lwith-lwhat);

Four points: First (and second), the length calculation (which
still omits room for the trailing '\0') is correct only for the
first replacement; after that, it fails to account for the size
increase from replacing earlier what's by longer with's. Third,
realloc() can fail just as malloc() can, and you should check for
this. Fourth, if realloc() fails and you store NULL in buffer,
you have just lost your only pointer to the still-allocated former
memory area; this is called a "memory leak."

memcpy(&buffer, with, lwith);
i += lwith-1;

This won't work unless lwhat==lwith. If they are unequal,
then the number of characters deposited in buffer is not the
same as the number of characters scanned in data; imagine
data="xx", what="x", with="supercalifragilisticexpialidocious".
You need two variables, one to keep track of the amount of
progress in each of the arrays.

} else memcpy(&buffer, &data, 1);

Why not a simple buffer=data? (But see above; it's
probably i and j, not i and i.)

memcpy(&buffer[ldata], 0, 1);

This is utterly wrong. It's trying to copy one character
from the spot indicated by the second argument to the spot
indicated by the first argument. What spot does the second
argument point to? Nowhere!

data = buffer;

This is legal, but ineffectual. The data variable is
local to the function, and changing it does not affect the
caller in any way. The caller's argument value initializes
the data parameter, but thereafter they are unconnected.

}

--------

Some doubts then:
1) malloc returns a pointer, no? so, the "buffer" var should contain
the address of the heap address, shouldn't it?
2) If so, memcpy wants a pointer as 1st and 2nd argument.. why should
i pass &buffer instead of simply buffer?

I have still some doubts about pointers... and reference/dereference
operators..

The comp.lang.c Frequently Asked Questions (FAQ) site
at <http://www.c-faq.com/> has good material on the topics
you're having trouble with; read sections 6, 4, and 5 in
conjunction with your textbook and things may become clearer.
Also see sections 7 and 8 -- in fact, you should at least
browse the entire FAQ, not to memorize what you find there
but to be aware of the content so you'll remember to return
to the FAQ next time you get into difficulty.