I have an array of pointers (char**) which contain some strings. The array ends with an empty string ('\0'). I am supposed to search for a specific word in that array of strings and delete the whole line (ofc using realloc and shortening the array of strings). I'm having hard time doing this, I keep getting 'bad ptr' error.
My code:
void deleteSentence(char **text){
char *word,*fptr;
int i=0;
word=(char*)calloc(BUFFER,sizeof(char));
printf("Enter word to delete sentences:\n");
gets(word);
while(text[i][0]!='\0'){
char *str=(char*)malloc((strlen(text[i])+1)*sizeof(char));
strcpy(str,text[i]);
fptr=strtok(str,DELIM);
while(fptr!=NULL){
if(strcmp(fptr,word)==0){
int j=i;
while(text[j][0]!='\0'){
text[j]=(char*)realloc(text[j],(strlen(text[j+1]))*sizeof(char));
strcpy(text[j],text[j+1]);
j++;
}
free(text[j]);
}
fptr=strtok(NULL,DELIM);
if(fptr!=NULL)
i++;
}
}
}
Help much appreciated :)
You're leaking memory like a sieve leaks water, and overrunning your arrays in at least two places. Furthermore the integration of input with the functional purpose of this code does literally nothing to help. The function should do one thing and one thing only:
Given a pointer to an array of
char*pointers terminated with an empty string (or NULL), delete all strings in the pointer array that contain word. The resulting potentially compacted array is the return value of the function.
Consider this:
char ** deleteSentances(char **text, const char *word)
{
char **dst = text, **src = text, **res = text;
size_t size = 1, deleted = 0;
// loop while we have a non-null string that isn't empty
while (*src && (*src)[0])
{
char *tmp = strdup(*src);
if (tmp == NULL)
{
perror("Failed to allocate tmp");
exit(EXIT_FAILURE);
}
char *token = strtok(tmp, DELIM);
// search for matching word
while (token && strcmp(word, token))
token = strtok(NULL, DELIM);
// if not found, keep the string. otherwise delete it.
if (!token)
{
*dst++ = *src++;
size++;
}
else
{
free(*src++);
++deleted;
}
// don't need this.
free(tmp);
}
// resize the original array (which could have only gotten smaller)
if (deleted > 0)
{
res = realloc(text, size * sizeof(*res));
if (res == NULL)
{
perror("Failed to allocate res");
exit(EXIT_FAILURE);
}
res[size-1] = *src;
}
return res;
}
Hopefully that explains enough. The code is called like this:
char **text, *word;
//... populate text with strings
//... populate word with prospect word
text = deleteSentances(text, word);
Memory Leaks O'Festival
The OP wanted to understand where memory leaks were in the original posted algorithm. Consider the following first and foremost: For every allocation, there should be a known point of free'ing that memory. This example is somewhat difficult to nail that concept down simply because you're bringing dynamic allocations to the function, and some of them are going to be kept.
That said, consider the following places of interest. We assume coming in to this that at some point we allocated a pointer-array of this form:
char **text = malloc(N * sizeof(*text));
I.e. we have N character points. In each of those, we further assume a dynamic allocation for a character string has also transpired:
for (int i=0; i<(N-1); ++i)
{
//... compute length of next string
text[i] = malloc(length * sizeof(**text));
//... copy in next string to text[i]
}
And finally, the last character pointer in the text array is either NULL or points to a dynamic string of length 0 (i.e. a 0-length terminated string).
Whew. Ok. after all of that lets look at your algorithm:
void deleteSentence(char **text)
{
char *word,*fptr;
int i=0;
// Leak 1: allocate a single buffer of BUFFER-length.
// this is never freed anywhere in this function
word=(char*)calloc(BUFFER,sizeof(char));
printf("Enter word to delete sentences:\n");
// Problem: gets() is so evil and bad it has been deprecated from
// the C langage and will not be available in the next release.
// use fgets() instead.
gets(word);
while(text[i][0]!='\0')
{
// Leak 2: Done N times, where N is the number of strings in
// your original array. again, this is never freed.
char *str=(char*)malloc((strlen(text[i])+1)*sizeof(char));
strcpy(str,text[i]);
fptr=strtok(str,DELIM);
while(fptr!=NULL)
{
if(strcmp(fptr,word)==0)
{
int j=i;
while(text[j][0]!='\0')
{
// Leak 3: Done M-N times for ever string we find in position
// M of the original array. This can be *huge* if there are
// a decent number of number of reductions that crunch your
// original array down.
text[j]=(char*)realloc(text[j],(strlen(text[j+1]))*sizeof(char));
strcpy(text[j],text[j+1]);
j++;
}
// Problem: this just freed the termination string, which should
// never be done. We now have undefined behavior for the rest
// of this algorithm since the terminatingg string is invalid.
free(text[j]);
// Problem: You shoud break right here. See below for why
}
// Problem: you're missing an else condition here. At this point
// if the strcmp() found a match there is no reason to continue
// the loop. You found a match and deleted the string, crunching
// all the other string down one slot in a most-inefficient
// memory-leaking algorihm.
fptr=strtok(NULL,DELIM);
// Problem: the logic here is completely wrong. The i in this case
// should be incremented OUTSIDE the inner while loop. Furthermore
// the test is backwards.
if(fptr!=NULL)
i++;
}
}
}
In short, if it were possible to leak more memory than you did in that algorithm, I'm hard pressed to see how. The posted code I provided will work given the confines of the description I presented, and should be carefully looked to, even stepped through with a debugger line-y-line, to better understand how it works.