Can anyone find the errors?

S

spacebebop

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

class X
{
public:
X();
X( char * );
~X();
void print();

private:
char *_str;
};

class Y : public X
{
public:
Y();
Y( char * );
~Y();
void print();

private:
char *_str;
};

X::X()
{
static char *str = "X's str empty";
_str = new char[ strlen( str ) ];
strcpy(_str, str);
cout << "X(), '" << str << "'\n";
}

X::X( char *str )
{
_str = new char[ strlen( str ) ];
strcpy(str, str );
cout << "X(char *), '" << str << "'\n";
}

X::~X()
{
cout << "~X(), deallocating '" << _str << "'\n";
delete _str;
}

void X::print()
{
cout << "X::print():\t'" << _str << "'\n";
}

Y::Y()
{
static char *str = "Y's str empty";
_str = new char[ strlen( str ) ];
strcpy(_str, str);
cout << "Y(), '" << str << "'\n";
}

Y::Y(char *str)
{
_str = new char[ strlen( str ) ];
strcpy( _str, str );
cout << "Y(char *), '" << str << "'\n";
}

Y::~Y()
{
cout << "~Y() deallocating '" << _str << "'\n";
delete _str;
}

void Y::print()
{
cout << "Y::print():\t'" << _str << "'\n";
}

int main()
{
X *x[3];

x[0] = new X( "X's data-index 0" );
x[1] = new Y( "Y's data-index 1" );

// Y y( "y's data-index 2" );
// x[2] = new Y( y );

x[0]->print();
x[1]->print();
// x[2]->print();

delete x[0];
delete x[1];
// delete x[2];

return 0;
}
 
T

Thomas J. Gritzan

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

class X
{
public:
X();
X( char * );
~X();
void print();

private:
char *_str;
};
[...more code...]
[Subject: Can anyone find the errors?]

Which one?

I guess, the biggest error is not using std::string.
 
R

Ron Natalie

(e-mail address removed) wrote:

Do you want to be more specific?

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

Using declarations are generally bad.
class X
{
public:
X();
X( char * );
~X();

Classes that require destructors probably also need copy constructors
and copy assignment operators (rule of three). If you had done
anything that would have invoked copy semantics for your class
you would have had further problems.
void print();

Did you perhaps also want to make this function virtual? Hard to tell
from your program.
private:
char *_str;
};
X::X()
{
static char *str = "X's str empty";
Deprecated conversion of const char array to char array.
_str = new char[ strlen( str ) ];
strcpy(_str, str);

WHOOP WHOOP... strcpy copies strlen(str) + 1 characters (the
null terminator is not counted by strlen. Undefined behavior
from writing off the end of the string.
cout << "X(), '" << str << "'\n";

ERROR. iostream does not necessarily define ostream functions.
You need said:
X::X( char *str )
{
_str = new char[ strlen( str ) ];
strcpy(str, str );

same bugs as before.
X::~X()
{
cout << "~X(), deallocating '" << _str << "'\n";
delete _str;

More undefined behavior.
Memory allocated by new[] must be freed with delete[].


Further, you'd have avoided many of these bugs if you'd
have just used std::string rather than making multiple
screw ups with char*.
 
R

Robert Bauck Hamar

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

class X
{
public:
X();
X( char * );

X(char const);
~X();
void print();

void print() const;
private:
char *_str;
};

class Y : public X
{
public:
Y();
Y( char * );

Y(char const *);
~Y();
void print();

void print() const;
private:
char *_str;

Hint: You already have a _str in X
};

X::X()
{
static char *str = "X's str empty";

static char str[] = "X's str empty";
_str = new char[ strlen( str ) ];

_str = new char[sizeof str];
strcpy(_str, str);

This copies strlen(str) + 1 characters! strlen() does not count the
terminating zero.

Why don't you use the std::string class?
cout << "X(), '" << str << "'\n";
}

X::X( char *str )

X::X(char const *str)
{
_str = new char[ strlen( str ) ];

_str = new char[strlen(str) + 1];
strcpy(str, str );
cout << "X(char *), '" << str << "'\n";
}

X::~X()
{
cout << "~X(), deallocating '" << _str << "'\n";
delete _str;
}

void X::print()

void X::print() const
{
cout << "X::print():\t'" << _str << "'\n";
}

Y::Y()

X::X() is called here. If you do not want this, you could do:

: X("Y's str empty")

[snip almost equal code]
int main()
{
X *x[3];

x[0] = new X( "X's data-index 0" );
x[1] = new Y( "Y's data-index 1" );

// Y y( "y's data-index 2" );
// x[2] = new Y( y );

x[0]->print();
x[1]->print();

This shall print "X::print()\tX's str empty\n"

http://www.parashift.com/c++-faq-lite/virtual-functions.html
// x[2]->print();

delete x[0];
delete x[1];

This line is undefined behaviour:

http://www.parashift.com/c++-faq-lite/virtual-functions.html#faq-20.7
// delete x[2];

return 0;
}
 
A

Alan Johnson

Ron said:
ERROR. iostream does not necessarily define ostream functions.
You need <ostream> included.

By this do you mean to imply that a minimal Hello world program using
the C++ IO library would require both <iostream> and <ostream>? That is:

#include <iostream>
#include <ostream>
int main()
{
std::cout << "Hello, world!\n";
}

My reading of the standard agrees with that interpretation. <iostream>
is needed to declare std::cout, but it is declared as extern, which
thanks to 7.1.1.8 means that class std::eek:stream need not be defined at
that point. Thus we must include <ostream> if we want to use operator<<
of class std::eek:stream.

This is somewhat surprising to me since there are literally thousands of
examples where only <iostream> is included to allow someone to use
std::cout. Even Stroustrup has such an example:
http://www.research.att.com/~bs/hello_world.c
 
O

Old Wolf

_str = new char[ strlen( str ) ];
strcpy(_str, str);

WHOOP WHOOP... strcpy copies strlen(str) + 1 characters (the
null terminator is not counted by strlen. Undefined behavior
from writing off the end of the string.
X::X( char *str )
{
_str = new char[ strlen( str ) ];
strcpy(str, str );

same bugs as before.

Actually it is different bugs. The second one
does not overflow _str because it never writes
to it. The undefined behaviour does not come
until this constructor is called with a string
literal as the argument:
x[0] = new X( "X's data-index 0" );

which I think is the first occurrence of UB
at runtime.
 
B

BobR

Alan Johnson said:
By this do you mean to imply that a minimal Hello world program using
the C++ IO library would require both <iostream> and <ostream>? That is:

#include <iostream>
#include <ostream>
int main(){
std::cout << "Hello, world!\n";
}

That is a "more correct" way of doing it.
My reading of the standard agrees with that interpretation. <iostream>
is needed to declare std::cout, but it is declared as extern, which
thanks to 7.1.1.8 means that class std::eek:stream need not be defined at
that point. Thus we must include <ostream> if we want to use operator<<
of class std::eek:stream.

This is somewhat surprising to me since there are literally thousands of
examples where only <iostream> is included to allow someone to use
std::cout.

99 out of 100 compiler implementations have both <istream> and <ostream>
included in <iostream> (, but not 'required').
It's that 1 that the wording in the standard goes out of it's way to not
exclude.
<G>

The <ostream> thing has been run-around-the-barn many times in this NG
alone.
 
J

James Kanze

By this do you mean to imply that a minimal Hello world program using
the C++ IO library would require both <iostream> and <ostream>? That is:
#include <iostream>
#include <ostream>
int main()
{
std::cout << "Hello, world!\n";
}

That's what the standard says.
My reading of the standard agrees with that interpretation. <iostream>
is needed to declare std::cout, but it is declared as extern, which
thanks to 7.1.1.8 means that class std::eek:stream need not be defined at
that point. Thus we must include <ostream> if we want to use operator<<
of class std::eek:stream.

In fact, the way I understood the intent was that <iostream>
really should just include <iosfwd> and the extern declarations.
The whole point of breaking the thing down into multiple headers
was that you don't pull in tons of lines that you don't need
each time. For whatever reasons, however, all (or at least all
I'm aware of) implementations do include <istream> and <ostream>
when you include <iostream>. So at the last reunion, the
committee voted to bring the standard in line with existing
implementations and most peoples expectations. Unless a future
This is somewhat surprising to me since there are literally
thousands of examples where only <iostream> is included to
allow someone to use std::cout. Even Stroustrup has such an
example:http://www.research.att.com/~bs/hello_world.c

I know. And those examples have conditionned user expectations,
and user expectations have conditioned implementations.
Personally, I would have prefered the reverse, that the
implementation only included the minimum, but it's too late now.
Although I'm not sure it's a good general principle, in this
case, I think it's best to align the standard with existing
practice.
 
B

bjarne

In fact, the way I understood the intent was that <iostream>
really should just include <iosfwd> and the extern declarations.
The whole point of breaking the thing down into multiple headers
was that you don't pull in tons of lines that you don't need
each time. For whatever reasons, however, all (or at least all
I'm aware of) implementations do include <istream> and <ostream>
when you include <iostream>. So at the last reunion, the
committee voted to bring the standard in line with existing
implementations and most peoples expectations. Unless a future


I know. And those examples have conditionned user expectations,
and user expectations have conditioned implementations.
Personally, I would have prefered the reverse, that the
implementation only included the minimum, but it's too late now.
Although I'm not sure it's a good general principle, in this
case, I think it's best to align the standard with existing
practice.

There is one more piece to this puzzle. At the time where the
standards text for <iostream> was written, essentially all code was
written using <iostream> and worked. The standard (unintentionally, as
far as I'm concerned, and surprisingly) broke that code. By making
code using just <iostream> legal, the standard was simply brought
*back* into line with reality.

-- Bjarne Stroustrup; http://www.research.att.com/~bs
 
M

Marcus Kwok

Ron Natalie said:
(e-mail address removed) wrote:

Do you want to be more specific?



Using declarations are generally bad.

Actually, this is a using *directive*. A using declaration would be
something like:

using std::cout;

Personally, I use using declarations in my .cpp files, but this is
largely personal preference so I will not expect to get into this
debate.

Obviously, using directives (and declarations as well) in headers are
bad, which is what I think you were trying to hint at.
 
J

James Kanze

There is one more piece to this puzzle. At the time where the
standards text for <iostream> was written, essentially all code was
written using <iostream> and worked. The standard (unintentionally, as
far as I'm concerned, and surprisingly) broke that code.

You mean that all code was written using <iostream.h>, I think.
There are two ways of viewing <iostream>: for whatever reasons,
I just considered it one more file in the group, designed to
allow access to a small subset of what was in the old
<iostream.h>, without pulling in a lot of other stuff. But a
lot of people (including yourself, I think) doubtlessly viewed it as
the
"new" version of <iostream.h>. All things considered, I rather
think that this is the more reasonable point of view, even if it
didn't occur to me at the time (and doesn't actually correspond
to the text in the standard---but as you say, it's likely that a
number of people who voted for it didn't realize this).
By making
code using just <iostream> legal, the standard was simply brought
*back* into line with reality.

That is, in fact, why I proposed it. Regardless of what it
"should have been", it's quite clear that in reality, it is a
direct replacement for <iostream.h>. Whether that was the
intent of the actual authors of the lines in the standard is
really pretty irrelevant today; it has become what it has
become, and whatever else it does, the standard shouldn't break
existing practice.
 

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,769
Messages
2,569,580
Members
45,054
Latest member
TrimKetoBoost

Latest Threads

Top