Differences in code implemented using this pointer and a variable THIS simulating this pointer

Discussion in 'C++' started by chikkubhai, Sep 27, 2007.

  1. chikkubhai

    chikkubhai Guest

    Why is the result different for the following set of two code snippets

    Code without using this pointer

    #include <string>
    #include <iostream>
    using namespace std;

    struct X {
    private:
    int len;
    char *ptr;
    public:
    int GetLen() {
    return len;
    }
    char * GetPtr() {
    return ptr;
    }
    X& Set(char *);
    X& Cat(char *);
    X& Copy(X&);
    void Print();
    };

    X& X::Set(char *pc) {
    len = strlen(pc);
    ptr = new char[len];
    strcpy(ptr, pc);
    return *this;
    }

    X& X::Cat(char *pc) {
    len += strlen(pc);
    strcat(ptr,pc);
    return *this;
    }

    X& X::Copy(X& x) {
    Set(x.GetPtr());
    return *this;
    }

    void X::print() {
    cout << ptr << endl;
    }

    int main() {
    X xobj1;
    xobj1.Set("abcd")
    .Cat("efgh");

    xobj1.Print();
    X xobj2;
    xobj2.Copy(xobj1)
    .Cat("ijkl");

    xobj2.Print();
    }

    Equivalent code, the THIS variable simulating the hidden use of this
    pointer

    #include <string>
    #include <iostream>
    using namespace std;

    struct X {
    private:
    int len;
    char *ptr;
    public:
    int GetLen (X* const THIS) {
    return THIS->len;
    }
    char * GetPtr (X* const THIS) {
    return THIS->ptr;
    }
    X& Set(X* const, char *);
    X& Cat(X* const, char *);
    X& Copy(X* const, X&);
    void Print(X* const);
    };

    X& X::Set(X* const THIS, char *pc) {
    THIS->len = strlen(pc);
    THIS->ptr = new char[THIS->len];
    strcpy(THIS->ptr, pc);
    return *THIS;
    }

    X& X::Cat(X* const THIS, char *pc) {
    THIS->len += strlen(pc);
    strcat(THIS->ptr, pc);
    return *THIS;
    }

    X& X::Copy(X* const THIS, X& x) {
    THIS->Set(THIS, x.GetPtr(&x));
    return *THIS;
    }

    void X::print(X* const THIS) {
    cout << THIS->ptr << endl;
    }

    int main() {
    X xobj1;
    xobj1.Set(&xobj1 , "abcd")
    .Cat(&xobj1 , "efgh");

    xobj1.Print(&xobj1);
    X xobj2;
    xobj2.Copy(&xobj2 , xobj1)
    .Cat(&xobj2 , "ijkl");

    xobj2.Print(&xobj2);
    }


    Both examples produce the following output:
    abcdefgh
    abcdefghijkl

    They are different for some reason. Any comments would be appreciated.
     
    chikkubhai, Sep 27, 2007
    #1
    1. Advertising

  2. chikkubhai

    Kai-Uwe Bux Guest

    chikkubhai wrote:

    > Why is the result different for the following set of two code snippets
    >
    > Code without using this pointer
    >
    > #include <string>
    > #include <iostream>
    > using namespace std;
    >
    > struct X {
    > private:
    > int len;
    > char *ptr;
    > public:
    > int GetLen() {
    > return len;
    > }
    > char * GetPtr() {
    > return ptr;
    > }
    > X& Set(char *);
    > X& Cat(char *);
    > X& Copy(X&);
    > void Print();
    > };
    >
    > X& X::Set(char *pc) {
    > len = strlen(pc);
    > ptr = new char[len];
    > strcpy(ptr, pc);
    > return *this;
    > }
    >
    > X& X::Cat(char *pc) {
    > len += strlen(pc);
    > strcat(ptr,pc);
    > return *this;
    > }
    >
    > X& X::Copy(X& x) {
    > Set(x.GetPtr());
    > return *this;
    > }
    >
    > void X::print() {
    > cout << ptr << endl;
    > }
    >
    > int main() {
    > X xobj1;
    > xobj1.Set("abcd")
    > .Cat("efgh");
    >
    > xobj1.Print();
    > X xobj2;
    > xobj2.Copy(xobj1)
    > .Cat("ijkl");
    >
    > xobj2.Print();
    > }
    >
    > Equivalent code, the THIS variable simulating the hidden use of this
    > pointer
    >
    > #include <string>
    > #include <iostream>
    > using namespace std;
    >
    > struct X {
    > private:
    > int len;
    > char *ptr;
    > public:
    > int GetLen (X* const THIS) {
    > return THIS->len;
    > }
    > char * GetPtr (X* const THIS) {
    > return THIS->ptr;
    > }
    > X& Set(X* const, char *);
    > X& Cat(X* const, char *);
    > X& Copy(X* const, X&);
    > void Print(X* const);
    > };
    >
    > X& X::Set(X* const THIS, char *pc) {
    > THIS->len = strlen(pc);
    > THIS->ptr = new char[THIS->len];
    > strcpy(THIS->ptr, pc);
    > return *THIS;
    > }
    >
    > X& X::Cat(X* const THIS, char *pc) {
    > THIS->len += strlen(pc);
    > strcat(THIS->ptr, pc);
    > return *THIS;
    > }
    >
    > X& X::Copy(X* const THIS, X& x) {
    > THIS->Set(THIS, x.GetPtr(&x));
    > return *THIS;
    > }
    >
    > void X::print(X* const THIS) {
    > cout << THIS->ptr << endl;
    > }
    >
    > int main() {
    > X xobj1;
    > xobj1.Set(&xobj1 , "abcd")
    > .Cat(&xobj1 , "efgh");
    >
    > xobj1.Print(&xobj1);
    > X xobj2;
    > xobj2.Copy(&xobj2 , xobj1)
    > .Cat(&xobj2 , "ijkl");
    >
    > xobj2.Print(&xobj2);
    > }
    >
    >
    > Both examples produce the following output:
    > abcdefgh
    > abcdefghijkl
    >
    > They are different for some reason. Any comments would be appreciated.


    Undefined behavior (for both programs). Your code has an off-by-one error in
    the allocation of the character buffer. You should use std::string.


    Best

    Kai-Uwe Bux
     
    Kai-Uwe Bux, Sep 27, 2007
    #2
    1. Advertising

  3. chikkubhai

    chikkubhai Guest

    I didn't quite get you, as I have already included the string class
    and compiled both code without any problem.
     
    chikkubhai, Sep 27, 2007
    #3
  4. chikkubhai

    chikkubhai Guest

    and using namespace std will take care of what you are mentioning in C+
    +
     
    chikkubhai, Sep 27, 2007
    #4
  5. chikkubhai

    Kai-Uwe Bux Guest

    chikkubhai wrote:

    [too little]

    Please quote enough context. This is not a chat room. Due to the news
    protocol, you cannot assume that previous post of the thread are available
    on the server.

    > I didn't quite get you, as I have already included the string class


    You have included the header <string>, but you did not use it. You used
    char* and the related functions. Probably, you meant to include <string.h>
    or <cstring>.

    > and compiled both code without any problem.


    That is because standard headers are allowed to pull other standard headers.
    In your implementation, either <string> or <iostream> seems include
    <cstring>. That is not guaranteed by the standard. Thus, your code has a
    bug there, too.

    As for the undefined behavior, look closely at the following lines:

    X& X::Set(char *pc) {
    len = strlen(pc);
    ptr = new char[len];
    strcpy(ptr, pc);
    return *this;
    }

    There is your main bug.


    Best

    Kai-Uwe Bux
     
    Kai-Uwe Bux, Sep 27, 2007
    #5
  6. chikkubhai

    chikkubhai Guest

    I did not quite get why you felt I was in a chat room. Take it easy.
    When you mentioned that I have to use std::string I replied saying I
    used using namespace std which will require me not to use std::string
    anymore.

    Anyways, what is or where is the off by one error? I still see the
    correct output as
    abcdefgh

    followed by

    abcdefghijkl
     
    chikkubhai, Sep 27, 2007
    #6
  7. chikkubhai

    Kai-Uwe Bux Guest

    chikkubhai wrote:

    > I did not quite get why you felt I was in a chat room.


    Because you behave like that. Please read the FAQ on netiquette in this
    forum (especially on quoting). You are the one asking for help. Show a
    little courtesy and follows local customs.

    > Take it easy.
    > When you mentioned that I have to use std::string I replied saying I
    > used using namespace std which will require me not to use std::string
    > anymore.


    Such recastings of conversations are better dealt with by quoting.

    Anyway, your point about std::string vs string is irrelevant since neither
    appeared in the code you posted. Instead of std::string, you used char*.
    When I say you should use std::string, I do not mean "std::string" instead
    of "string" (and I could not have meant that, because there was no "string"
    in your code), I meant use std::string instead of char*.

    > Anyways, what is or where is the off by o
    > ne error?



    X& X::Set(char *pc) {
    len = strlen(pc);
    ptr = new char[len]; // <--- here, you are short one character
    strcpy(ptr, pc);
    return *this;
    }


    But fixing that, will still not make it work. There are much bigger issues:


    > #include <string>


    should read

    #include <cstring>

    > #include <iostream>
    > using namespace std;
    >
    > struct X {
    > private:
    >   int len;
    >   char *ptr;
    > public:
    >   int GetLen() {
    >     return len;
    >   }
    >   char * GetPtr() {
    >     return ptr;
    >   }


    Poor design: this allows client code to write beyond allocated memory.
    Buffer overruns will come from this.

    >   X& Set(char *);
    >   X& Cat(char *);
    >   X& Copy(X&);
    >   void Print();
    > };


    Your class does not disable copy-construction and assignment. However, it
    handles neither correctly. Your Set method allocates memory that is never
    freed. The class leaks memory big time.

    >
    > X& X::Set(char *pc) {
    >   len = strlen(pc);
    >   ptr = new char[len];


    Here is the off by 1 error.

    Also, should Set() be called twice on the same object, memory allocated in
    the first run is not freed.

    >   strcpy(ptr, pc);
    >   return *this;
    > }
    >
    > X& X::Cat(char *pc) {
    >   len += strlen(pc);
    >   strcat(ptr,pc);
    >   return *this;
    > }


    This is even worse: you append beyond allocated memory.

    >
    > X& X::Copy(X& x) {
    >   Set(x.GetPtr());
    >   return *this;
    > }
    > void X::print() {
    >   cout << ptr << endl;
    > }
    >
    > int main() {
    >   X xobj1;
    >   xobj1.Set("abcd")
    >        .Cat("efgh");
    >
    >   xobj1.Print();
    >   X xobj2;
    >   xobj2.Copy(xobj1)
    >        .Cat("ijkl");
    >
    >   xobj2.Print();
    > }
    >


    All in all, you should not touch char*. There is no need to deal with char*
    anyway since there is std::string. You should use it.


    > I still see the
    > correct output as
    > abcdefgh
    >
    > followed by
    >
    > abcdefghijkl


    Undefined behavior sometimes looks correct and sometimes looks wrong.


    Best

    Kai-Uwe Bux
     
    Kai-Uwe Bux, Sep 27, 2007
    #7
  8. chikkubhai

    chikkubhai Guest

    On Sep 27, 12:01 am, Kai-Uwe Bux <> wrote:
    > chikkubhai wrote:
    > > I did not quite get why you felt I was in a chat room.

    >
    > Because you behave like that. Please read the FAQ on netiquette in this
    > forum (especially on quoting). You are the one asking for help. Show a
    > little courtesy and follows local customs.
    >
    > > Take it easy.
    > > When you mentioned that I have to use std::string I replied saying I
    > > used using namespace std which will require me not to use std::string
    > > anymore.

    >
    > Such recastings of conversations are better dealt with by quoting.
    >
    > Anyway, your point about std::string vs string is irrelevant since neither
    > appeared in the code you posted. Instead of std::string, you used char*.
    > When I say you should use std::string, I do not mean "std::string" instead
    > of "string" (and I could not have meant that, because there was no "string"
    > in your code), I meant use std::string instead of char*.
    >
    > > Anyways, what is or where is the off by o
    > > ne error?

    >
    > X& X::Set(char *pc) {
    > len = strlen(pc);
    > ptr = new char[len]; // <--- here, you are short one character
    > strcpy(ptr, pc);
    > return *this;
    > }
    >
    > But fixing that, will still not make it work. There are much bigger issues:
    >
    > > #include <string>

    >
    > should read
    >
    > #include <cstring>
    >
    > > #include <iostream>
    > > using namespace std;

    >
    > > struct X {
    > > private:
    > > int len;
    > > char *ptr;
    > > public:
    > > int GetLen() {
    > > return len;
    > > }
    > > char * GetPtr() {
    > > return ptr;
    > > }

    >
    > Poor design: this allows client code to write beyond allocated memory.
    > Buffer overruns will come from this.
    >
    > > X& Set(char *);
    > > X& Cat(char *);
    > > X& Copy(X&);
    > > void Print();
    > > };

    >
    > Your class does not disable copy-construction and assignment. However, it
    > handles neither correctly. Your Set method allocates memory that is never
    > freed. The class leaks memory big time.
    >
    >
    >
    > > X& X::Set(char *pc) {
    > > len = strlen(pc);
    > > ptr = new char[len];

    >
    > Here is the off by 1 error.
    >
    > Also, should Set() be called twice on the same object, memory allocated in
    > the first run is not freed.
    >
    > > strcpy(ptr, pc);
    > > return *this;
    > > }

    >
    > > X& X::Cat(char *pc) {
    > > len += strlen(pc);
    > > strcat(ptr,pc);
    > > return *this;
    > > }

    >
    > This is even worse: you append beyond allocated memory.
    >
    >
    >
    >
    >
    > > X& X::Copy(X& x) {
    > > Set(x.GetPtr());
    > > return *this;
    > > }
    > > void X::print() {
    > > cout << ptr << endl;
    > > }

    >
    > > int main() {
    > > X xobj1;
    > > xobj1.Set("abcd")
    > > .Cat("efgh");

    >
    > > xobj1.Print();
    > > X xobj2;
    > > xobj2.Copy(xobj1)
    > > .Cat("ijkl");

    >
    > > xobj2.Print();
    > > }

    >
    > All in all, you should not touch char*. There is no need to deal with char*
    > anyway since there is std::string. You should use it.
    >
    > > I still see the
    > > correct output as
    > > abcdefgh

    >
    > > followed by

    >
    > > abcdefghijkl

    >
    > Undefined behavior sometimes looks correct and sometimes looks wrong.
    >
    > Best
    >
    > Kai-Uwe Bux


    wooow, those were a lot of help/suggestions to me dude. It will take
    me awhile to even understand your comments as I am not advanced and do
    not have experience to be able to point out mistakes that you could.
    Thanks and I appreciate your help and time.

    I will learn how to recast or requote as I have never done that
    previously and will surely read the netiquette on this forum soon.
     
    chikkubhai, Sep 27, 2007
    #8
  9. chikkubhai

    Kai-Uwe Bux Guest

    chikkubhai wrote:

    > On Sep 27, 12:01 am, Kai-Uwe Bux <> wrote:
    >> chikkubhai wrote:
    >> > I did not quite get why you felt I was in a chat room.

    >>
    >> Because you behave like that. Please read the FAQ on netiquette in this
    >> forum (especially on quoting). You are the one asking for help. Show a
    >> little courtesy and follows local customs.
    >>
    >> > Take it easy.
    >> > When you mentioned that I have to use std::string I replied saying I
    >> > used using namespace std which will require me not to use std::string
    >> > anymore.

    >>
    >> Such recastings of conversations are better dealt with by quoting.
    >>
    >> Anyway, your point about std::string vs string is irrelevant since
    >> neither appeared in the code you posted. Instead of std::string, you used
    >> char*. When I say you should use std::string, I do not mean "std::string"
    >> instead of "string" (and I could not have meant that, because there was
    >> no "string" in your code), I meant use std::string instead of char*.
    >>
    >> > Anyways, what is or where is the off by o
    >> > ne error?

    >>
    >> X& X::Set(char *pc) {
    >> len = strlen(pc);
    >> ptr = new char[len]; // <--- here, you are short one character
    >> strcpy(ptr, pc);
    >> return *this;
    >> }
    >>
    >> But fixing that, will still not make it work. There are much bigger
    >> issues:
    >>
    >> > #include <string>

    >>
    >> should read
    >>
    >> #include <cstring>
    >>
    >> > #include <iostream>
    >> > using namespace std;

    >>
    >> > struct X {
    >> > private:
    >> > int len;
    >> > char *ptr;
    >> > public:
    >> > int GetLen() {
    >> > return len;
    >> > }
    >> > char * GetPtr() {
    >> > return ptr;
    >> > }

    >>
    >> Poor design: this allows client code to write beyond allocated memory.
    >> Buffer overruns will come from this.
    >>
    >> > X& Set(char *);
    >> > X& Cat(char *);
    >> > X& Copy(X&);
    >> > void Print();
    >> > };

    >>
    >> Your class does not disable copy-construction and assignment. However, it
    >> handles neither correctly. Your Set method allocates memory that is never
    >> freed. The class leaks memory big time.
    >>
    >>
    >>
    >> > X& X::Set(char *pc) {
    >> > len = strlen(pc);
    >> > ptr = new char[len];

    >>
    >> Here is the off by 1 error.
    >>
    >> Also, should Set() be called twice on the same object, memory allocated
    >> in the first run is not freed.
    >>
    >> > strcpy(ptr, pc);
    >> > return *this;
    >> > }

    >>
    >> > X& X::Cat(char *pc) {
    >> > len += strlen(pc);
    >> > strcat(ptr,pc);
    >> > return *this;
    >> > }

    >>
    >> This is even worse: you append beyond allocated memory.
    >>
    >>
    >>
    >>
    >>
    >> > X& X::Copy(X& x) {
    >> > Set(x.GetPtr());
    >> > return *this;
    >> > }
    >> > void X::print() {
    >> > cout << ptr << endl;
    >> > }

    >>
    >> > int main() {
    >> > X xobj1;
    >> > xobj1.Set("abcd")
    >> > .Cat("efgh");

    >>
    >> > xobj1.Print();
    >> > X xobj2;
    >> > xobj2.Copy(xobj1)
    >> > .Cat("ijkl");

    >>
    >> > xobj2.Print();
    >> > }

    >>
    >> All in all, you should not touch char*. There is no need to deal with
    >> char* anyway since there is std::string. You should use it.
    >>
    >> > I still see the
    >> > correct output as
    >> > abcdefgh

    >>
    >> > followed by

    >>
    >> > abcdefghijkl

    >>
    >> Undefined behavior sometimes looks correct and sometimes looks wrong.
    >>
    >> Best
    >>
    >> Kai-Uwe Bux

    >
    > wooow, those were a lot of help/suggestions to me dude. It will take
    > me awhile to even understand your comments as I am not advanced and do
    > not have experience to be able to point out mistakes that you could.
    > Thanks and I appreciate your help and time.


    Ok, here is a version of your code using std::string.

    #include <string>
    #include <iostream>
    using namespace std;

    struct X {
    private:

    std::string data;

    public:

    // note the const
    // we promise that calling this function will not
    // modify the object.
    int GetLen() const {
    return data.length();
    }

    // note the const in the return type:
    // this way, the client cannot overwrite it.
    char const * GetCstring() const {
    return data.c_str();
    }

    // note the const in the parameter:
    // we promise that we will not modify the
    // string passed. The compiler will check that.
    X& Set(char const *);
    X& Cat(char const *);


    // X& Copy(X&);
    // this really should be an assignment operator copy constructor.
    /*
    X& operator= ( X const & rhs ) {
    data = other.data;
    return ( *this );
    }
    void Print();
    };
    */
    // However, the compiler generates that one for free.

    void Print() const;

    };

    X& X::Set( char const * pc) {
    data = pc;
    return *this;
    }

    X& X::Cat(char const * pc) {
    data.append( pc );
    return *this;
    }

    void X::print() const {
    cout << data << endl;
    }

    int main() {
    X xobj1;
    xobj1.Set("abcd")
    .Cat("efgh");

    xobj1.Print();
    X xobj2;
    ( xobj2 = xobj1 )
    .Cat("ijkl");
    xobj2.Print();
    }


    You should do the following experiment. Run this version and you version in
    a memory checked environment, e.g., use valgrind (it's a terrific tool!).


    If you are in for an interesting learning experience, you can try rewriting
    that class using char* instead of std::string.

    E.g.:

    class X {

    char * data_ptr;

    public:

    ....

    };


    However, it will be difficult:

    a) You have to write a default constructor. The member data_ptr will
    otherwise point nowhere meaningfull and the object starts off with an
    inconsistent state. BadIdea(tm)

    b) You have to manually manage the memory every time the length of the
    string potentially changes, i.e., in the Set(), Cat() and operator=()
    method.

    c) You have to provide a destructor that frees the memory when an object of
    type X goes out of scope.


    The library class std::string takes care of all those issues behind the
    scenes.


    Should you decide to recode class X, you should run all your tests in
    valgrind and make sure that your code is memory clean. Whenever there is a
    question, post what you have, and most likely you will get some help and
    code review here.


    > I will learn how to recast or requote as I have never done that
    > previously and will surely read the netiquette on this forum soon.


    That's very good. I am looking forward to your postings.


    Best

    Kai-Uwe Bux
     
    Kai-Uwe Bux, Sep 27, 2007
    #9
  10. chikkubhai

    James Kanze Guest

    On Sep 27, 2:25 am, Kai-Uwe Bux <> wrote:
    > chikkubhai wrote:
    > > Why is the result different for the following set of two code snippets


    > > Code without using this pointer


    > > #include <string>
    > > #include <iostream>
    > > using namespace std;


    > > struct X {
    > > private:
    > > int len;
    > > char *ptr;
    > > public:
    > > int GetLen() {
    > > return len;
    > > }
    > > char * GetPtr() {
    > > return ptr;
    > > }
    > > X& Set(char *);
    > > X& Cat(char *);
    > > X& Copy(X&);
    > > void Print();
    > > };


    > > X& X::Set(char *pc) {
    > > len = strlen(pc);
    > > ptr = new char[len];
    > > strcpy(ptr, pc);
    > > return *this;
    > > }


    > > X& X::Cat(char *pc) {
    > > len += strlen(pc);
    > > strcat(ptr,pc);
    > > return *this;
    > > }


    > > X& X::Copy(X& x) {
    > > Set(x.GetPtr());
    > > return *this;
    > > }


    > > void X::print() {
    > > cout << ptr << endl;
    > > }


    > > int main() {
    > > X xobj1;
    > > xobj1.Set("abcd")
    > > .Cat("efgh");


    > > xobj1.Print();
    > > X xobj2;
    > > xobj2.Copy(xobj1)
    > > .Cat("ijkl");
    > > xobj2.Print();
    > > }


    > > Equivalent code, the THIS variable simulating the hidden use of this
    > > pointer

    [...]
    > > Both examples produce the following output:
    > > abcdefgh
    > > abcdefghijkl


    > > They are different for some reason. Any comments would be appreciated.


    > Undefined behavior (for both programs). Your code has an off-by-one error in
    > the allocation of the character buffer. You should use std::string.


    It's more than off by one; look at the function Cat.

    It's also a pretty wierd class that uses dynamic memory, but
    doesn't have a user defined constructor, assignment operator or
    destructor.

    --
    James Kanze (GABI Software) email:
    Conseils en informatique orientée objet/
    Beratung in objektorientierter Datenverarbeitung
    9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
     
    James Kanze, Sep 27, 2007
    #10
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. StanMan
    Replies:
    1
    Views:
    423
    quantum_dot
    Jun 5, 2007
  2. Andrea Taverna (Tavs)
    Replies:
    17
    Views:
    1,253
    Andrea Taverna (Tavs)
    Sep 24, 2008
  3. Home_Job_opportunity
    Replies:
    0
    Views:
    503
    Home_Job_opportunity
    Jan 8, 2009
  4. Home_Job_opportunity
    Replies:
    0
    Views:
    589
    Home_Job_opportunity
    Jan 14, 2009
  5. Replies:
    19
    Views:
    313
Loading...

Share This Page