Can anyone find the errors?

Discussion in 'C++' started by spacebebop@gmail.com, Jun 27, 2007.

  1. Guest

    #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;
    }
    , Jun 27, 2007
    #1
    1. Advertising

  2. wrote:
    > #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.

    --
    Thomas
    http://www.netmeister.org/news/learn2quote.html
    Thomas J. Gritzan, Jun 27, 2007
    #2
    1. Advertising

  3. Ron Natalie Guest

    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 <ostream> included.

    > 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*.
    Ron Natalie, Jun 27, 2007
    #3
  4. wrote:

    > #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;
    > }


    --
    rbh
    Robert Bauck Hamar, Jun 27, 2007
    #4
  5. Alan Johnson Guest

    Ron Natalie wrote:
    > wrote:
    >> cout << "X(), '" << str << "'\n";

    >
    > 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

    --
    Alan Johnson
    Alan Johnson, Jun 28, 2007
    #5
  6. Old Wolf Guest

    On Jun 28, 10:01 am, Ron Natalie <> wrote:
    > wrote:
    > > _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.
    Old Wolf, Jun 28, 2007
    #6
  7. BobR Guest

    Alan Johnson <> wrote in message...
    > Ron Natalie wrote:
    > > wrote:
    > >> cout << "X(), '" << str << "'\n";

    > >
    > > 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";
    > }


    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.

    --
    Bob R
    POVrookie
    BobR, Jun 28, 2007
    #7
  8. James Kanze Guest

    On Jun 28, 1:23 am, Alan Johnson <> wrote:
    > Ron Natalie wrote:
    > > wrote:
    > >> cout << "X(), '" << str << "'\n";


    > > 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";
    > }


    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
    vote rescends this decision, the #include <ostream> will not be
    necessary in the next version of the standard.

    > 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.

    --
    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, Jun 28, 2007
    #8
  9. bjarne Guest

    On Jun 28, 6:42 am, James Kanze <> wrote:
    >
    > 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
    > vote rescends this decision, the #include <ostream> will not be
    > necessary in the next version of the standard.
    >
    > > 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. EvenStroustruphas 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.
    >


    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
    bjarne, Jun 28, 2007
    #9
  10. Marcus Kwok Guest

    Ron Natalie <> wrote:
    > wrote:
    >
    > Do you want to be more specific?
    >
    >
    >> #include <cstring>
    >> #include <iostream>
    >> using namespace std;

    >
    > 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.

    --
    Marcus Kwok
    Replace 'invalid' with 'net' to reply
    Marcus Kwok, Jun 28, 2007
    #10
  11. James Kanze Guest

    On Jun 28, 3:21 pm, bjarne <> wrote:
    > On Jun 28, 6:42 am, James Kanze <> wrote:


    > > 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
    > > vote rescends this decision, the #include <ostream> will not be
    > > necessary in the next version of the standard.


    > > > 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. EvenStroustruphas 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.


    > 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.

    --
    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, Jun 29, 2007
    #11
    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. Mark Goldin

    Errors, errors, errors

    Mark Goldin, Jan 17, 2004, in forum: ASP .Net
    Replies:
    2
    Views:
    942
    Mark Goldin
    Jan 17, 2004
  2. Jonathan Folland
    Replies:
    0
    Views:
    407
    Jonathan Folland
    Apr 1, 2005
  3. ad
    Replies:
    2
    Views:
    855
  4. Wybo Dekker
    Replies:
    1
    Views:
    352
    Yukihiro Matsumoto
    Nov 15, 2005
  5. vdvorkin
    Replies:
    0
    Views:
    401
    vdvorkin
    Feb 10, 2011
Loading...

Share This Page