On Sep 27, 12:01 am, Kai-Uwe Bux <jkherci...@gmx.net> 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.