the strcpy function keeps crashing

Question

#include <iostream>
using namespace std;

defining class

class fancyString {
private:
    char *content;
    bool flag_bold;
    bool flag_italics;
public:
    fancyString(){
        content="";
        flag_bold= false;
        flag_italics=false;
    }

in both functions I'm asked to use the old fashioned calloc

    fancyString(char* cntnt){
        content=(char *) calloc(strlen(cntnt)+1, sizeof(char*));

Usually the strcpy is the main reason of the crash

        strcpy(cntnt,content);
    }
    fancyString(fancyString & f1){
        content=(char *) calloc(strlen(f1.content)+1, sizeof(char*));

Usually the strcpy is the main reason of the crash

           strcpy(f1.content,content);
        flag_bold=f1.flag_bold;
        flag_italics=f1.flag_italics;
    }
   friend ostream& operator<<(ostream& os, const fancyString& FS){
       os<<"string is "<<FS.content<<endl<<"bold status is "<<FS.flag_bold<<endl<<"italics status is "<<FS.flag_italics<<endl;
       return os;
    }
    ~fancyString(){
       cout << "Destroying the string\n";
        if ( content != NULL )
            free (content);
    }
};

main function

int main(int argc, const char * argv[]) {

    fancyString fs1 ("First Example");
     fancyString fs2(fs1);
    cout<<fs2;
    return 0;
}

Your code blows up on this: `int main() { fancyString fs1("First"); fancyString fs2; fs2 = fs1; }` — PaulMcKenzie, Jan 14 '20 at 21:06
The compiler should have at least warned you that passing a string literal as `char*` argument is deprecated or forbidden (since C++11). You must take string literals as `const char*`. This would have avoided the main issue pointed out in the answer as well. — walnut, Jan 14 '20 at 21:08
Don't you get a warning about converting a string literal to `char*` ? Also pay attention to source and destination in `strcpy`. Both information together should explain the issue. — Michael Doubez, Jan 14 '20 at 21:11

Adrian Mole · Answer 1 · 2020-01-14T21:37:17.520

4

You have the arguments to the strcpy calls the wrong way round! See the definition on cppreference:

char * strcpy ( char * destination, const char * source );

So, in your your first constructor, the call:

strcpy(cntnt,content);

is attempting to copy the newly-allocated buffer into the passed argument, which is actually a (constant) string literal:

int main(int argc, const char * argv[]) {
   fancyString fs1 ("First Example");
//...

NOTES based on suggestions made in the comments:

(1) Note that, in your calloc call - which is allocating an 'array' of char, the elements' size is sizeof(char) not sizeof(char*) (which would be appropriate for an 'array' of pointers). Use:

    content = (char *) calloc(strlen(cntnt)+1, sizeof(char));

and similarly in the copy constructor.

(2) Assuming that your first constructor is never intended to modify the string given as its argument, you should really specify it as const:

fancyString(const char* cntnt){
  //...

(This would have flagged the error of having the strcpy arguments wrong!)

Please feel free to ask for further clarification and/or explanation.

edited Jan 14 '20 at 21:37

answered Jan 14 '20 at 21:06

Adrian Mole

49,934
160
51
83

3

May be not directly related to the crash, but better point out wrong `calloc` usage as well i.e. sizeof(char) instead of sizeof(char*). – aep Jan 14 '20 at 21:08
@aep Well spotted! Are you happy for me to include your comment as an edit to my post? – Adrian Mole Jan 14 '20 at 21:10
@kirolos morcos The arguments order follows the assignment operator arguments order: destination = source. – zdf Jan 14 '20 at 21:10
@aep Or rather advice not to use `calloc` at all since its use is technically UB and `new char[]()` is type safe and does the same. – walnut Jan 14 '20 at 21:10
@AdrianMole sure. – aep Jan 14 '20 at 21:11
@walnut Maybe OP is duty bound: **in both functions I'm asked to use the old fashioned calloc** – Adrian Mole Jan 14 '20 at 21:12
@walnut Very true, but unfortunately he has been advised(_wrongly) to use *old fashioned* `calloc`. – aep Jan 14 '20 at 21:13
@AdrianMole Oh, I didn't see that. Doesn't seem like they are really taught C++ then, though. – walnut Jan 14 '20 at 21:13
1

I would add to the answer that: 1) implicit const literal to char* conversion is for C compatibility and introduces UB (like any const_cast) and 2) warning should not be disregarded (go practice while learning is to treat them as error - `-Werror` in gcc) – Michael Doubez Jan 14 '20 at 21:26
As an aside, OP should use `strdup()` (defined by POSIX and others equivalent to `extern "C" auto strdup(const char* s) noexcept { auto n = strlen(s) + 1; auto r = (char*)malloc(n); if(r) memcpy(r, s, n); return r; }`). – Deduplicator Jan 14 '20 at 22:00

the strcpy function keeps crashing

1 Answers1