I have code that is supposed to remove all characters in one C-string from another.

The problem arises when I try to use the function strstr: both an array and a char* get converted to bool. Obviously it doesn't work because strstr needs to receive 2 char* arguments.

#include <iostream>
#include <ctype.h>
#include <string.h>
#include <malloc.h>

using namespace std;

char* temp_char(char* orig,char* del) 
{            char *str_temp, *temp, *p, *c;
            char symbol[2];
            int i=0;
            int len = strlen(orig);
            temp=(char *)calloc(len,1);
            str_temp = (char *)calloc(len,1);
            strcpy(str_temp,orig);
            
            for(i=0;i<strlen(del);i  ){                
                symbol[0]=del[i];
                symbol[1]=0;
                temp[0]=0;
                 while( p=strstr(str_temp,del)!=NULL){
                  strncat(temp,str_temp,p-str_temp);
                  p  ;
                    str_temp=p;
                 }
             strcat(temp,str_temp);    
            strcpy(str_temp,temp);     
            }
                cout<<temp;
    return temp;
}
int main () {
char a[]="stuccsess",b[]="test";
 
 temp_char(a,b);}    

Any help would be appreciated.

CodePudding user response：

You have two errors in one line of your code. The first is not addressing the issue that the != operator has higher priority than =. Thus, in the following line:

while( p=strstr(str_temp,del)!=NULL) {

the comparison is actually:

while( p = ( strstr(str_temp,del)!=NULL ) ){

So, you are attempting to assign the result of the != test, which will be a bool value, to a char* variable (p).

You can fix this, easily, by putting the assignment expression in parentheses.

However, although that will fix the compiler error, it will not address the second error. Just a few lines above that while statement, you assign one of the characters of the del string to the first of the symbol string (and, properly, add a nul terminator to that) … but you never then actually use that symbol string. Instead, you pass the entire del string as the second argument in the call to strstr. You should be passing your created symbol as that second argument.

So, changing that while line to the following will make the code work:

while ((p = strstr(str_temp, symbol)) != nullptr) { // Use nullptr in C  

But that leaves other errors in your function. How will you ever be able to free the memory allocated in the str_temp = (char*)calloc(len, 1); call? Once you have subsequently modified that str_temp pointer (in the str_temp = p; line), you no longer have the original pointer value, which must be used when calling free(). So, you need to save that value, just after you have made the allocation. (The temp memory pointer is returned, so that can be freed in the main function.)

There are other issues in your code that could be improved, like using new[] and delete[] in C , rather than the old, C-language calloc, and that your index and length variables should really be of size_t type, rather than int. Here's a version with the corrections and suggestions discussed applied:

#include <iostream>
#include <cstring>

using std::cout;

char* temp_char(char* orig, char* del)
{
    size_t len = strlen(orig);
    char* temp = new char[len   1];
    char* str_temp = new char[len   1];
    char* save_temp = temp; // Save it so we can call delete...
    strcpy(str_temp, orig);

    for (size_t i = 0; i < strlen(del); i  ) {
        char symbol[2];
        symbol[0] = del[i];
        symbol[1] = 0;
        temp[0] = 0;
        char* p;
        while ((p = strstr(str_temp, symbol)) != nullptr) {
            strncat(temp, str_temp, static_cast<size_t>(p - str_temp));
            p  ;
            str_temp = p;
        }
        strcat(temp, str_temp);
        strcpy(str_temp, temp);
    }
    cout << temp;
    delete[] save_temp; // ... don't forget to free this.
    return temp;
}

int main()
{
    char a[] = "stuccsess", b[] = "test";
    char* answer = temp_char(a, b);
    delete[] answer; // We should free the memory allocated!
    return 0;
}

But your approach is far more complicated than it need be. You can simply loop through the original string and check each character to see if it is in the 'delete' string (using the strchr function, for example); if it is not (i.e. that strchr returns nullptr), then append the character to the accumulated temp and increase the running length:

char* temp_char(char* orig, char* del)
{
    size_t len = strlen(orig);
    char* temp = new char[len   1];
    int newlen = 0;
    for (char* cp = orig; *cp != '\0';   cp) {
        if (strchr(del, *cp) == nullptr) temp[newlen  ] = *cp;
    }
    temp[newlen] = '\0'; // Add null terminator
    std::cout << temp;
    return temp;
}