read() and write() may fail, we do not check for this

[daniel-j-h]

All our reads via stream.read(...) are unchecked. That is, in case of extraction failures or premature eof, we silently go on with uninitialized data structures (hint: undefined behavior anyone?).

Note: ticket explains for read, same applies to write.

Two solutions are possible:

• check the read function's return value (stream&) via its operator bool conversion
• let streams throw on error (not on per default, to mock with developers)

int main() try {
  std::ifstream in{"bin"};
  assert(in);
  // in.exceptions(std::ifstream::failbit | std::ifstream::badbit | std::ifstream::eofbit);
  // --^

  std::vector<char> v(10);
  auto& rc = in.read(v.data(), 10);

  if(!rc) {
  // --^
    if(rc.eof()) 
      return 10;
    if(rc.bad()) 
      return 11;
    if(rc.fail()) 
      return 12;
  }

} catch(const std::exception& e) {
  std::cerr << e.what() << std::endl;
}

Discussion: I think we can automate this to a certain degree with a clever substitution to turn

x.read(y,z);

into

ensure(x.read(y,z));

relying on the returned stream's conversion to bool in a boolean context, with ensure being a assert-like function that also triggers in release builds and potentially throws.

References:

• http://en.cppreference.com/w/cpp/io/basic_istream/read
• http://en.cppreference.com/w/cpp/io/ios_base/iostate

[daniel-j-h]

Updated ticket, same applies for writing. As of now, this is the state we're in:

$ rg '\.read\((.*),(.*)\);' |wc -l
62
$ rg '\.write\((.*),(.*)\);' |wc -l
50

[danpat]

We're partway there with #3383 and #3321

$ ag '\.read\((.*),(.*)\);' |wc -l
       5
$ ag '\.write\((.*),(.*)\);' | wc -l
      51