How to overload std::ofstream::put()?

Question

I want to write int16_t values to file.

Therefore I tried to overload the std::ofstream::put() method.

#include <fstream>
#include <cstdint>

class Ofstream : public std::ofstream
{
public:
    Ofstream( const std::string & s) : std::ofstream(s) {}

    // for little-endian machines
    Ofstream & put(int16_t val)
    {
        char lsb, msb;
        lsb = (char)val;
        val >>= 8;
        msb = (char)val;
        put(lsb) && put(msb);
        return *this;
    }
    ~Ofstream() {}
};
int main()
{
    int16_t val = 0x1234;
    Ofstream ofile( "test");
    ofile.put(val);
}

At this I always get a Segmentation fault, so what's wrong with?

I think your version of `put` hides the base class version, so you get a stack overflow. Easy to check this with a debugger. But πάνταῥεῖ is right, this isn't the way to do this. — john, Jun 29 '19 at 08:51
AS john noted, your version of `put()` hides the inherited one. So the calls of `put()` within your `put()` are calling itself recursively. The result is infinite recursion. Inheritance from standard stream classes is a bad idea - better to use containment (i.e. the `ostream` as a member of your class, not a base). — Peter, Jun 29 '19 at 09:05
I suspect the OP is expecting the base class `put(char)` to be invoked rather than the `put(int16_t)` implementation in the derived class because it's technically a better match (were it not for being hidden). — WhozCraig, Jun 29 '19 at 09:19
@WhozCraig - yeah. Too bad that expectation is an example of exactly what the hiding rule is intended to prevent. — Peter, Jun 29 '19 at 09:22

score 1 · Accepted Answer · answered Jun 29 '19 at 09:13

1

Your put() function calls itself rather than the base class version. So you get infinite recursion, which leads to stack overflow.

Replace

put(lsb) && put(msb);

with

std::ofstream::put(lsb) && std::ofstream::put(msb);

answered Jun 29 '19 at 09:13

Sid S

6,037
2
18
24

3

This solution presumes, incorrectly, that inheriting from `std::ofstream` is a good idea. – Peter Jun 29 '19 at 09:19
2

@Peter, No, it doesn't. – Sid S Jun 29 '19 at 09:36
As a side note: From what I understood in OP's question, they probably want to have numbers in the output file not characters. – πάντα ῥεῖ Jun 29 '19 at 10:22
2

Why should deriving from std::i/o/f/s/stream be not a good idea? All methods will be further available besides my own added stuff. I think that stuff of the standard library is made for customization if needed. – Nico Schumann Jun 29 '19 at 11:30
@NicoSchumann So see my answer. This would solve it. You can always consider to change your accept mark at any time. – πάντα ῥεῖ Jun 29 '19 at 11:44
1

@NicoSchumann - the reason it's not a good idea to inherit from a fair number of types in the standard library is that they are designed in a way that works against using them as a base class. A discussion of some aspects is at https://stackoverflow.com/questions/1073958/extending-the-c-standard-library-by-inheritance Furthermore, running afoul of the hiding rule - as you are - is often an indication that your approach is problematical (there are cases where it is appropriate to work around the hiding rule, but if that was your situation you wouldn't have needed to ask the question). – Peter Jun 29 '19 at 13:56

πάντα ῥεῖ · Answer 2 · 2019-06-29T20:10:04.163

1

The main problems with your code (infinite recursive calls), were already answered correctly.

Using explicit scoping like

std::ofstream::put(lsb) && std::ofstream::put(msb);

will fix this.

I want to write int16_t values to file.

Though I am under the impression that you want to write binary numbers in network byte order (big endian) to the file, and not put characters as text, and that's not what you're finally trying to achieve.

Here's how I would approach that (independently of the current machine architecture):

#include <fstream>
#include <arpa/inet.h>

struct Ofstream {
    std::ofstream os;

    Ofstream( const std::string & s) : os(s,std::ios_base::binary) {}

    void put(uint16_t dt) {
        uint16_t netdt = htons(dt);
        os.write((char*)&netdt,sizeof(netdt))
    }
};

int main() {
    uint16_t val = 0x1234;
    Ofstream ofile("test");
    ofile.put(val);
}

In general it's not a good idea to inherit from standard library classes unless they are explicitely intended to do so for implementation (i.e. std::ostream).
Rather use them as member variables.

edited Jun 29 '19 at 20:10

answered Jun 29 '19 at 11:15

πάντα ῥεῖ

1
13
116
190

Thanks, but I'm unversed with the arpa library. Also, I think that it would be good, staying with the std library if there exists a solution. And I need the values for fitting a VM, which I'm writing at the time (just for fun, as a hobby project). – Nico Schumann Jun 29 '19 at 11:58
@NicoSchumann The networking libraries and `htonx()` / `ntohx()` functions are available at almost all operating systems (would be `winsock.h` for windows). It's a not so easy task to write a conversion to network byte order working with all host architectures. It's more portable to do it this way. – πάντα ῥεῖ Jun 29 '19 at 12:00
@πάνταῥεῖ, The problem he was asking for help with, was what was causing his program to crash. What you have addressed here is a different - smaller - problem that had nothing to do with what he asked. Soliciting a change of "best answer" tag to an answer that does nothing to answer the question asked is dishonest at best. – Sid S Jun 29 '19 at 19:53
@SidS Well, that's the curse of decades of experience and having a broader view of issues. Note that I mentioned that the primary problem was already addressed in the existing answers / comments. Also note that I left a comment on your answer what the **rea**l problem is. Complain elsewhere please. – πάντα ῥεῖ Jun 29 '19 at 20:00
@SidS BTW, what hinders you to address the core problem beyond fixing the obvious flaw in your answer? There's no _dishonesty_ from my side, I cant see it. – πάντα ῥεῖ Jun 29 '19 at 20:16

How to overload std::ofstream::put()?

2 Answers2