1

The following code compiles and works correctly on g++ 4.7.2-5 on Debian 7.

#include <iostream>
#include <string.h>

using namespace std;

class mystring {
  char * buf;
  static char * dupbuf(const char * buf) {
    char * result = new char[strlen(buf) + 1];
    strcpy(result, buf);
    return result;
  }

  public:
    mystring(const char * o)
    : buf(dupbuf(o)) {}

    ~mystring() { delete[] buf; }
    mystring(const mystring &) = delete;
    mystring & operator=(const mystring&) = delete;

    void write(ostream & o) const {
      if (!buf) { exit(1); } // remove me
      o << buf;
    }
};

ostream & operator <<(ostream & o, const mystring & ms) {
  ms.write(o);
};

int main() {
    mystring m("hello");
    cout << m << endl;
    return 0;
}

...unless you compile with -O2 or above. Then it segfaults, and valgrind claims an invalid read from 0x0. I'm guessing there's some stack corruption, but for the life of me I can't find it.

Interestingly, removing the line marked "remove me" makes the problem disappear. So does adding an endl to write. Any ideas?

lutzky
  • 598
  • 5
  • 12
  • 2
    That's what you get for using manual memory management. – Bartek Banachewicz Aug 04 '14 at 11:17
  • 3
    does "court" not give error msg??? – Rahul Aug 04 '14 at 11:21
  • 2
    That's what you deserve for not following the "Law of Three", too. – Ulrich Eckhardt Aug 04 '14 at 11:26
  • It's hard to type code on a phone (don't ask...) – lutzky Aug 04 '14 at 11:29
  • Added code showing it's not a rule of three problem. As for manual memory management, you're right of course, this is a simplified example from a larger program where I encountered this, and manual memory management made more sense. It should still work... – lutzky Aug 04 '14 at 11:33
  • @lutkzy your bug in this code is not a rule of three problem, however you will run into such problems if you do anything else with this class. If you're not ready to give your class correct value semantics, at least disable the copy-constructor and copy-assignment operator so you do not have surprise bugs. – M.M Aug 04 '14 at 11:55
  • @Matt You are, of course, correct. I did do that, but omitted it from this post to save on typing (...from mobile). Laziness has not proven to be an effective strategy this time... – lutzky Aug 04 '14 at 15:53

1 Answers1

9
ostream & operator <<(ostream & o, const mystring & ms) {
  ms.write(o);
};

This causes undefined behaviour because it doesn't return anything. Also, the empty declaration at namespace level (";") is completely unnecessary (and used to be illegal in older C++ standards).

Ulrich Eckhardt
  • 16,572
  • 3
  • 28
  • 55
  • 1
    in C++ `;` is an empty declaration, which is legal – M.M Aug 04 '14 at 11:31
  • I'm pretty sure it is not legal everywhere. `if(x);` is legal, but at namespace scope, I believe it isn't. – Ulrich Eckhardt Aug 04 '14 at 11:34
  • I'm pretty sure it is; the C++ grammar allows `;` to be an empty declaration (and therefore fine at namespace scope), however in C `;` can only be an empty statement so it can only occur at block scope. – M.M Aug 04 '14 at 11:40
  • The comments in https://stackoverflow.com/questions/3849900/semicolon-in-namespace-necessary indicate that you are right and that I'm still in legacy C++ mode. (: – Ulrich Eckhardt Aug 04 '14 at 11:50
  • 1
    I see.. I didn't realize it was only added in C++11, seems we both learnt something :) – M.M Aug 04 '14 at 11:53
  • 1
    The worst part is that had I compiled with `-Wall` like I tell everyone else to do, it would have caught this. I should have my geek license revoked. – lutzky Aug 04 '14 at 12:39