Return to Answer

I agree with you that this would be more intuitive to use by chaining streams, rather than acting as a queue that must be pushed into and pulled out of. I've never written a filtering stream like that myself, but I think you want to construct an ostream with a custom streambuf for each filter.

I definitely think that separating the line encoding and the block packing would be a good thing, and would allow your unit tests to be much more selective, and therefore more diagnostic.

We seem to be assuming these typedefs:

using std::uint32_t;
using std::uint8_t;

Reviewing the main() - it's quite restrictive to insist on two filenames (and that the output file be seekable). It would be more natural if it was willing to use standard i/o streams if no arguments are given.

    uint32_t sum{id};
    auto n{datasize/sizeof(uint32_t)};
    for (uint32_t *ptr = reinterpret_cast<uint32_t *>(&data); n; ++ptr) {
        sum += *ptr;
        --n;
    }
    return sum;

This looks like a candidate for std::span:

    std::span as_u32{reinterpret_cast<std::uint32_t*>(data.begin()),
                     reinterpret_cast<std::uint32_t*>(data.end())};
    return std::accumulate(as_u32.begin(), as_u32.end(), std::uint32_t{});

Or, using a simple pair of iterators, for C++17 and earlier:

    auto first = reinterpret_cast<std::uint32_t*>(data.begin());
    auto last = reinterpret_cast<std::uint32_t*>(data.end());
    return std::accumulate(first, last, std::uint32_t{});

We do have a problem here in that the data are interpreted as std::uint32_t in the endianness of the host. That means that different platforms can generate different checksums, something generally considered undesirable.

I agree with you that this would be more intuitive to use by chaining streams, rather than acting as a queue that must be pushed into and pulled out of. I've never written a filtering stream like that myself, but I think you want to construct an ostream with a custom streambuf for each filter.

I definitely think that separating the line encoding and the block packing would be a good thing, and would allow your unit tests to be much more selective, and therefore more diagnostic.

We seem to be assuming these typedefs:

using std::uint32_t;
using std::uint8_t;

Reviewing the main() - it's quite restrictive to insist on two filenames (and that the output file be seekable). It would be more natural if it was willing to use standard i/o streams if no arguments are given.

    uint32_t sum{id};
    auto n{datasize/sizeof(uint32_t)};
    for (uint32_t *ptr = reinterpret_cast<uint32_t *>(&data); n; ++ptr) {
        sum += *ptr;
        --n;
    }
    return sum;

This looks like a candidate for std::span:

    std::span as_u32{reinterpret_cast<std::uint32_t*>(data.begin()),
                     reinterpret_cast<std::uint32_t*>(data.end())};
    return std::accumulate(as_u32.begin(), as_u32.end(), std::uint32_t{});

Or, using a simple pair of iterators, for C++17 and earlier:

    auto first = reinterpret_cast<const std::uint32_t*>(data.begin());
    auto last = reinterpret_cast<const std::uint32_t*>(data.end());
    return std::accumulate(first, last, std::uint32_t{});

This method should probably be declared const.

We do have a problem here in that the data are interpreted as std::uint32_t in the endianness of the host. That means that different platforms can generate different checksums, something generally considered undesirable.

I agree with you that this would be more intuitive to use by chaining streams, rather than acting as a queue that must be pushed into and pulled out of. I've never written a filtering stream like that myself, but I think you want to construct an ostream with a custom streambuf for each filter.

I definitely think that separating the line encoding and the block packing would be a good thing, and would allow your unit tests to be much more selective, and therefore more diagnostic.

We seem to be assuming these typedefs:

using std::uint32_t;
using std::uint8_t;

Reviewing the main() - it's quite restrictive to insist on two filenames (and that the output file be seekable). It would be more natural if it was willing to use standard i/o streams if no arguments are given.

    uint32_t sum{id};
    auto n{datasize/sizeof(uint32_t)};
    for (uint32_t *ptr = reinterpret_cast<uint32_t *>(&data); n; ++ptr) {
        sum += *ptr;
        --n;
    }
    return sum;

This looks like a candidate for std::span:

    std::span as_u32{reinterpret_cast<std::uint32_t*>(data.begin()),
                     reinterpret_cast<std::uint32_t*>(data.end())};
    return std::accumulate(as_u32.begin(), as_u32.end(), std::uint32_t{});

We do have a problem here in that the data are interpreted as std::uint32_t in the endianness of the host. That means that different platforms can generate different checksums, something generally considered undesirable.

I agree with you that this would be more intuitive to use by chaining streams, rather than acting as a queue that must be pushed into and pulled out of. I've never written a filtering stream like that myself, but I think you want to construct an ostream with a custom streambuf for each filter.

I definitely think that separating the line encoding and the block packing would be a good thing, and would allow your unit tests to be much more selective, and therefore more diagnostic.

We seem to be assuming these typedefs:

using std::uint32_t;
using std::uint8_t;

Reviewing the main() - it's quite restrictive to insist on two filenames (and that the output file be seekable). It would be more natural if it was willing to use standard i/o streams if no arguments are given.

    uint32_t sum{id};
    auto n{datasize/sizeof(uint32_t)};
    for (uint32_t *ptr = reinterpret_cast<uint32_t *>(&data); n; ++ptr) {
        sum += *ptr;
        --n;
    }
    return sum;

This looks like a candidate for std::span:

    std::span as_u32{reinterpret_cast<std::uint32_t*>(data.begin()),
                     reinterpret_cast<std::uint32_t*>(data.end())};
    return std::accumulate(as_u32.begin(), as_u32.end(), std::uint32_t{});

Or, using a simple pair of iterators, for C++17 and earlier:

    auto first = reinterpret_cast<std::uint32_t*>(data.begin());
    auto last = reinterpret_cast<std::uint32_t*>(data.end());
    return std::accumulate(first, last, std::uint32_t{});

We do have a problem here in that the data are interpreted as std::uint32_t in the endianness of the host. That means that different platforms can generate different checksums, something generally considered undesirable.

I agree with you that this would be more intuitive to use by chaining streams, rather than acting as a queue that must be pushed into and pulled out of. I've never written a filtering stream like that myself, but I think you want to construct an ostream with a custom streambuf for each filter.

I definitely think that separating the line encoding and the block packing would be a good thing, and would allow your unit tests to be much more selective, and therefore more diagnostic.

We seem to be assuming these typedefs:

using std::uint32_t;
using std::uint8_t;

Reviewing the main() - it's quite restrictive to insist on two filenames (and that the output file be seekable). It would be more natural if it was willing to use standard i/o streams if no arguments are given.

    uint32_t sum{id};
    auto n{datasize/sizeof(uint32_t)};
    for (uint32_t *ptr = reinterpret_cast<uint32_t *>(&data); n; ++ptr) {
        sum += *ptr;
        --n;
    }
    return sum;

This looks like a candidate for std::span:

    std::span as_u32{reinterpret_cast<std::uint32_t*>(data.begin()),
                     reinterpret_cast<std::uint32_t*>(data.end())};
    return std::accumulate(as_u32.begin(), as_u32.end(), std::uint32_t{});

I agree with you that this would be more intuitive to use by chaining streams, rather than acting as a queue that must be pushed into and pulled out of. I've never written a filtering stream like that myself, but I think you want to construct an ostream with a custom streambuf for each filter.

I definitely think that separating the line encoding and the block packing would be a good thing, and would allow your unit tests to be much more selective, and therefore more diagnostic.

We seem to be assuming these typedefs:

using std::uint32_t;
using std::uint8_t;

Reviewing the main() - it's quite restrictive to insist on two filenames (and that the output file be seekable). It would be more natural if it was willing to use standard i/o streams if no arguments are given.

    uint32_t sum{id};
    auto n{datasize/sizeof(uint32_t)};
    for (uint32_t *ptr = reinterpret_cast<uint32_t *>(&data); n; ++ptr) {
        sum += *ptr;
        --n;
    }
    return sum;

This looks like a candidate for std::span:

    std::span as_u32{reinterpret_cast<std::uint32_t*>(data.begin()),
                     reinterpret_cast<std::uint32_t*>(data.end())};
    return std::accumulate(as_u32.begin(), as_u32.end(), std::uint32_t{});

We do have a problem here in that the data are interpreted as std::uint32_t in the endianness of the host. That means that different platforms can generate different checksums, something generally considered undesirable.