[libcxx-commits] [PATCH] D122257: [libc++] Fix undefined behavior in `std::filebuf`

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Mar 28 12:00:17 PDT 2022


ldionne added inline comments.


================
Comment at: libcxx/test/std/input.output/file.streams/fstreams/filebuf.assign/nonmember_swap_min.pass.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
Could we also add this in `libcxx/test/std/input.output/file.streams/fstreams/filebuf.assign/nonmember_swap.pass.cpp`:

```
// lhs uses a small buffer, rhs is empty
{
    std::filebuf f;
    assert(f.open(temp.c_str(), std::ios_base::out | std::ios_base::in
                                            | std::ios_base::trunc) != 0);
    assert(f.is_open());
    assert(f.sputn("123", 3) == 3);
    f.pubseekoff(1, std::ios_base::beg);
    assert(f.sgetc() == '2');
    std::filebuf f2;
    swap(f, f2);
    assert(!f.is_open());
    assert(f2.is_open());
    assert(f2.sgetc() == '2');
}
```

This basically reverses the order of arguments to `swap` in the test that is already there. Perhaps we should also add a test that checks for swapping two filebufs that are not using the small buffer?


================
Comment at: libcxx/test/std/input.output/file.streams/fstreams/filebuf.assign/nonmember_swap_min.pass.cpp:32
+{
+    std::string tmp = get_temp_file_name();
+    std::string tmpA = get_temp_file_name();
----------------
I don't think you ever use `tmp`, I think you could remove it altogether.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122257/new/

https://reviews.llvm.org/D122257



More information about the libcxx-commits mailing list