[WIP] - Retyping #152
[WIP] - Retyping #152
Conversation
dr7ana
commented
Dec 4, 2024
dr7ana
commented
- Based on 0RTT, Early data, session ticketing, key verification, stateless reset #147
dr7ana
force-pushed
the
retyping
branch
4 times, most recently
from
97d80ec to
b61203e
Compare
- By passing `opt::disable_key_verification` in calls to `Endpoint::{listen,connect}(...)` the application can decide whether to use key verification for inbound or outbound connections.
- can be toggled on/off, bespoke types for handling generation and storage - improvements, better handling, callbacks
- ping binaries added to test timing - more thoughtful compile time checking
The copy and move constructors/operators are a decent amount of complexity keeping track of pointers that really isn't needed because we nearly always want these held inside a unique_ptr, and so it simplifies things to just enforce that everywhere and fix the two occurances that held in a value and then sort of mutated into a unique_ptr. This also lets us make the key data const, which gives us better safety assurances for using it as the key of a map (as we currently are). Also makes the private fields actually private, along with the constructors (to enforce all usage now go through make). Also dropped some unused comparison & hashing functions.
- added RAII gnutls_datum object, used where needed. Must be careful, can double free if the memory is owned by another object or held within ngtcp2 - misc fixes from oxen-io#147
- cmake configuration to force fmt > 10 when compiling with clang-19 - misc
| using cspan = oxenc::const_span<char>; | ||
| using uspan = oxenc::const_span<unsigned char>; | ||
| using bspan = oxenc::const_span<std::byte>; |
There was a problem hiding this comment.
The more I see this const_span typedef outside oxenc itself, the more I don't like it.
oxenc::const_span<C> is just a typedef for std::span<const C>, but it doesn't seem like a useful typedef for external use: it's both longer to type than the thing it aliases; and it introduces a level of apparent, but not actual, differentiation that seems to have negative value. If you're familiar with std::span but not oxenc details, then std::span<const char> is instantly understood, but oxenc::const_span<C> isn't until you look it up and see that it's just a bit of indirection. And if you are familiar with oxenc internals, then you're almost certainly familiar with std::span. So who benefits from it?
In the origin WIP PR that added const_span, it wasn't just a typedef but was its own class, but that didn't survive to merging. Within oxen-encoding itself, I suppose it has some marginal utility as declaring "this is the span that oxenc uses", but outside oxenc I'm having trouble seeing a point.
Is there some higher purpose or future intention that I'm missing here?
There was a problem hiding this comment.
That's a valid argument, and IIRC we talked about all the pros/cons of the suboptimal set of choices for approaching this. Between all the options, an inline namespaced typedef was the least intrusive, and still allowed us to declare comparators for it.
it's both longer to type than the thing it aliases; and it introduces a level of apparent, but not actual, differentiation that seems to have negative value
I think one of the benefits we deliberated on was precisely that ability to implement type-safe code -- operators, comparators, etc -- around it. I remember that you had elaborated on that point, but again this was a few months ago so apologies if my recollection isn't perfect. In terms of the name length, that's why I like {c,u,b}span as application typedefs. In terms of the negative value, I suppose I'm struggling to see the explicit negatives here. The concerns you're mentioning are salient and a continuation of a lot of what we talked about, but if I'm missing the specific negatives or what this typedef gets in the way of doing, could you bring me along?
Within oxen-encoding itself, I suppose it has some marginal utility as declaring "this is the span that oxenc uses", but outside oxenc I'm having trouble seeing a point
I would surmise that by importing <oxenc/span.h>, there's the understanding that the operators and comparators will come with it. That would be a specific The comparators alone have an idiosyncratic implementation, since the whole "it's equal if the pointers are the same" is not what we were wanting.
The nomenclature also indicates the purpose behind its use contextually, which is a less impactful point but I think one worth mentioning nonetheless.
