boostorg / endian

Boost Endian library
45 stars 49 forks source link

Add non-const data() method to endian_buffer #25

Closed cy20lin closed 4 years ago

cy20lin commented 6 years ago

Allow users directly read data into endian_buffer through non-const data() method.

As described in the docs, endian_buffer is designed to be memcpyable. It is quite reasonable for users to pretend that the buffer point by non-const endian_buffer data() method would also be non-const. And may thus come up with following attempts, trying to access endian_buffer.

    boost::endian::big_int32_buf_t i(123);
    boost::endian::big_int32_buf_t j(456);

    // works
    std::cout.write(i.data(), sizeof(i));

    // doesn't work, since istream::read requires i.data() to be a pointer to non-const char
    std::cin.read(i.data(), sizeof(i));

    // works as describe in docs
    std::memcpy(&i, &j, sizeof(i));

    // doesn't work, same reason above
    std::memcpy(i.data(), j.data(), sizeof(i));

It feels like there are some inconsistencies when it comes to accessing endian_buffer.

This is just my humble opinion though. I think it would be better to add non-const data() method into endian_buffer.

Or maybe there are some further design decisions that I don't take into considerations?

Kojoley commented 5 years ago

This seems to me as a reasonable feature. @pdimov can you please trigger the CI?

pdimov commented 5 years ago

It is a reasonable feature.

Tangentially related, I don't like the char const* return type of data() though. Would much rather return unsigned char const*, but I'm not sure how much code this is going to break, so I'm soliciting opinions.

Kojoley commented 5 years ago

Tangentially related, I don't like the char const return type of data() though. Would much rather return unsigned char const, but I'm not sure how much code this is going to break, so I'm soliciting opinions.

I also was surprised by the fact that it returns char type when I first saw it. From my point of view changing it to unsigned char should not be a big problem, because no one must rely on char type representation and in all valid uses there already should be a reinterpret_cast in user code that either casts destination to char const* or a return value of data() to unsigned char const*. In the first case there need a little effort to make it work and to establish portability, in the second case no actions required form the user.

pdimov commented 4 years ago

I've added the non-const data overloads and made data return unsigned char* (on develop.)