Use std::span()

[KindDragon]

I like library interface very much, but it should use std::span(void *)/std::span(const void *) in lot of methods instead of pair (void *buf, size_t n)/(const void *buf, size_t n)

[fpagliughi]

Hey, thanks for the suggestion. I know after 10 years this still claims to be a "modern" C++ library, but it is actually compatible with C++14. I still have a lot of legacy projects that use it, and they will likely never have a compiler upgrade. I was considering updating to C++17 in a near-future release, but no plans for C++20 at the moment.

It doesn't seem worth it to phase out a lot of legacy and embedded code for small changes like pair vs span, even if they would be nice. But maybe some conditional compilation based on the compiler version? I don't know if that would get confusing.

[fpagliughi]

Ha. It's been a while since I was deep into this library, that I forget the implementation details! I grep'ed through the code and it seems I'n not using std::pair<> anywhere in the library.

The term "pair" is used in the library to return to a pair of sockets derived from the C socket library call socketpair():
https://man7.org/linux/man-pages/man2/socketpair.2.html

Those are actually returned in C++ in a 2-element tuple, std::tuple<socket,socket> - not a std::pair<>.

I think span wouldn't work here since it's a non-owning container, right?

[KindDragon]

I think best way is to provide you own simplified implementation (only with std::size_t Extent = std::dynamic_extent) for C++ 14- C++17 that have same interface with std::span, but when C++ compiler support C++20 you could use instead alias for std::span. We use this approach in our project.

[KindDragon]

Or use span from MS GSL https://github.com/microsoft/GSL/blob/main/include/gsl/span

[KindDragon]

I think span wouldn't work here since it's a non-owning container, right?

Yes, span is not owning. It just array slice :), but with helper methods that allow to get sub-span that often very helpful

[fpagliughi]

Could you give me an example, specifically, of what you think should be changed?

[KindDragon]

class stream_socket : public socket
{
....
    virtual ssize_t read(std::span<void> buf);
    virtual ioresult read_r(std::span<void> buf);
    virtual ssize_t read_n(std::span<void> buf);
    virtual ioresult read_n_r(std::span<void> buf);
    virtual ssize_t write(std::span<const void> buf);
    virtual ioresult write_r(std::span<const void> buf);
    virtual ssize_t write_n(std::span<const void> buf);
    virtual ioresult write_n_r(std::span<const void> buf);
};

So you could use
sockpp::tcp_connector conn({host, port});
std::array<char, 10> buffer;
conn.write(buffer);
or
conn.write(std::span(buffer).first(3)); // write part of buffer

[fpagliughi]

Oh, OK. Now I get you.

I would not want to break backward compatibility to replace the existing functions, but maybe we can add those with conditional compilation for C++20?

class stream_socket : public socket
{
    ...
    virtual ssize_t read(void *buf, size_t n);
    virtual ioresult read_r(void *buf, size_t n);
    virtual ssize_t read_n(void *buf, size_t n);
    ...

#if __cplusplus >= 202002L
    virtual ssize_t read(std::span<void> buf) { return read(buf.data(), buf.size()); }
    virtual ioresult read_r(std::span<void> buf) { return read_r(buf.data(), buf.size()); }
    virtual ssize_t read_n(std::span<void> buf) { return read_n(buf.data(), buf.size()); }
    ...
#endif
};

[KindDragon]

Yeah, it's ok. But probably also support ms GSL span implementation for C++14-17.

BTW read should user std::span<const void> :)

[fpagliughi]

I could see it maybe as an opt-in CMake build option. It's not something I would have the time or inclination to do in the near future, but if you send over a PR, I'd give it a look.

[andrewauclair]

std::span<void> isn't valid. The proper way to do this stuff in C++17 is std::span<std::byte>. std::byte is the "correct" type to use for collections of bytes.

[fpagliughi]

Yes, of course you're right. It should not be a span of void!

But honestly, I'm not yet convinced of std::byte especially for collections of data (as opposed to individual bit fields for machine register, etc).. I'm still in the uint8_t camp.

But that also shows the problem with using something like span<> for this. How easy/difficult is it to convert from different array and collection types? Is that assumed you can/should do that with a span of bytes? The problem is that a void* means "anything", but a void means "nothing". What a language.

[andrewauclair]

This is exactly what std::byte is for. Its underlying type is unsigned char and is not a character type or an arithmetic type.

I guess std::span<std::byte> is locked in more than void*. Definitely would be an API break. People are too used to void* for sockets.