How is this implementation of inheritance of C++ fstream for convienient binary output causing failure opening the file?

Question

Code outputs, "file not open."

union converter_t {
  char byte[4];
  short int word[2];
  int dword[1];
} converter;

class enhancedfstream: public fstream {
  public:
    enhancedfstream(string s, _Ios_Openmode iosom) {
     fstream(s, iosom);
    }
    void write(int i) {
      converter.dword[0] = i;
      fstream::write(converter.byte, 4);
    }
    void write(short int si) {
      converter.word[0] = si;
      fstream::write(converter.byte, 2);
    }
    void write(string s) {
      char* tca = &s[0];
      fstream::write(tca, s.length());
    }
    void write(char c) {
      // intended correction:
      converter.byte[0] = c;
      fstream::write(converter.byte, 1);
      // as original, before correction:
      // fstream::write(new char[1] {c}, 1);
    }
    void write(char* cp, int i) {
      fstream::write(cp, i);
    }
};

  enhancedfstream fs(fn, fstream::out | fstream::binary | fstream::trunc);
  if (fs.is_open()) {} else cout << "file not open\n";

Linux filesystem review from shell shows empty file creation succeeds, but programatically, the file appears as unopened and subsequent writes are of no effect. Compilation and execution are done as root within a subdirectory of the root directory. Compiled with emulated g++ (Alpine 10.3.1_git20210424) 10.3.1 20210424 (C++14) (copyright 2020) in ipadOs iSH 1.3.2 (build 494).

Is the inheritance as implemented above done improperly, or is fstream peculiar in how it can successfully be subclassed? Is a buffer setup going unattended to? Standard reported fstream opening failure causes seem to be absent, unless a null buffer is causing the opening failure. Non-subclassed fstream with the same specs opens fine.

Not the answer, but using `new char[1] {c}` like this leaks memory. — HolyBlackCat, Aug 21 '23 at 03:45
[Unions and type-punning](https://stackoverflow.com/questions/25664848/unions-and-type-punning) may be worth a read. — Retired Ninja, Aug 21 '23 at 04:38
In C++ do not use `union` use `std::variant` (read the C++ standard on union behavior, and you will not be so happy to use it UB everywhere). But even then, what you are doing is not really safe anyway. I also do not see the need to inherit from fstream, just make some free functions (in a namespace) that accept a stream as first input parameter — Pepijn Kramer, Aug 21 '23 at 05:57
In general binary serialization is harder then you might think, are you even sure another system that is going to read your file has the exact same memory layout, endianess etc. as the system that wrote your file? Just use a tested binary serialization library if you want to read/write binary data. — Pepijn Kramer, Aug 21 '23 at 05:59
@ Pepjin Kramer No. Had not at all been planning on implementing this code ever on any other system. — reallyhuh, Aug 21 '23 at 20:35

Ted Lyngmo · Accepted Answer · 2023-08-21T05:05:41.043

1

You construct a temporary fstream in your constructor. That temporary fstream is closed at the end of the full expression.

fstream(s, iosom); // opened and closed immediately

Also, _Ios_Openmode is an internal implementation specific type. Do not use those.

Use the member initializer list to initialize the base class:

enhancedfstream(const std::string& s, std::ios_base::openmode iosom)
    : std::fstream(s, iosom) // now opened correctly
{
    // empty constructor body
}

Demo

Sidenote:s

fstream::write(new char[1] {c}, 1); is not a good idea. It'll allocate memory that is never released. Use
```
write(&c, 1);
```
or simply
```
put(c);
```
The global converter doesn't do anything to simplify the implementation. It does however make it unsafe to use your enhancedfstream in multiple threads at the same time.
Don't hardcode the sizes of int and short int. They are not always 4 and 2 bytes.

edited Aug 21 '23 at 05:05

answered Aug 21 '23 at 03:56

Ted Lyngmo

93,841
5
60
108

@reallyhuh As written it does compile just fine. [Demo](https://godbolt.org/z/qWz71P33T) – Ted Lyngmo Aug 21 '23 at 04:53
Had things out of order here. Main solution mentioned there worked as it was *actually written. – reallyhuh Aug 21 '23 at 05:00
@reallyhuh Great! Glad it worked out! – Ted Lyngmo Aug 21 '23 at 05:00
And the int and char sizes and all of this code is not meant at all to be anything but implementation specific, along with the little endian approach to data storage. (Please don't hack me, or pass me along to those who will? Neither!) – reallyhuh Aug 21 '23 at 05:16
@reallyhuh If you ask if the demo site can be trusted, then I would say _Yes_. It's one of the most commonly used (if not _the_ most commonly used) by people here on SO when it comes to sharing short demos. _"And the int and char sizes and all of this code is not meant at all to be anything but implementation specific"_ - Ok, I'd still use `sizeof` to show intent. Also, if you want to generalize it using a function template later, you'll have most bits in place. – Ted Lyngmo Aug 21 '23 at 05:22

How is this implementation of inheritance of C++ fstream for convienient binary output causing failure opening the file?

1 Answers1