-
Notifications
You must be signed in to change notification settings - Fork 526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve C++ wrapper. #3769
Improve C++ wrapper. #3769
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3769 +/- ##
==========================================
- Coverage 75.59% 75.57% -0.02%
==========================================
Files 56 56
Lines 15554 15554
==========================================
- Hits 11758 11755 -3
- Misses 3796 3799 +3 |
src/inc/msquic.hpp
Outdated
static | ||
QUIC_STATUS | ||
QUIC_API | ||
NoOpCallback( | ||
_In_ MsQuicListener* /* Listener */, | ||
_In_opt_ void* /* Context */, | ||
_Inout_ QUIC_LISTENER_EVENT* /* Event */ | ||
) noexcept { | ||
return QUIC_STATUS_SUCCESS; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix formatting and we need to return an error by default, otherwise it would just accept a connection and leak it.
static | |
QUIC_STATUS | |
QUIC_API | |
NoOpCallback( | |
_In_ MsQuicListener* /* Listener */, | |
_In_opt_ void* /* Context */, | |
_Inout_ QUIC_LISTENER_EVENT* /* Event */ | |
) noexcept { | |
return QUIC_STATUS_SUCCESS; | |
} | |
static | |
QUIC_STATUS | |
QUIC_API | |
NoOpCallback( | |
_In_ MsQuicListener* /* Listener */, | |
_In_opt_ void* /* Context */, | |
_Inout_ QUIC_LISTENER_EVENT* /* Event */ | |
) noexcept { | |
return QUIC_STATUS_NOT_SUPPORTED; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, I'm not sure it ever makes sense to have a no-op callback for a listener. Maybe we just remove it and always require a callback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I will remove this an also update the default parameters in the MsQuicListener c'tor.
src/inc/msquic.hpp
Outdated
}; | ||
|
||
typedef QUIC_STATUS MsQuicListenerCallback( | ||
_In_ struct MsQuicListener* listener, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_In_ struct MsQuicListener* listener, | |
_In_ struct MsQuicListener* Listener, |
src/perf/lib/PerfServer.cpp
Outdated
@@ -129,7 +129,7 @@ PerfServer::GetExtraData( | |||
|
|||
QUIC_STATUS | |||
PerfServer::ListenerCallback( | |||
_In_ HQUIC /*ListenerHandle*/, | |||
_In_ MsQuicListener* /*ListenerHandle*/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_In_ MsQuicListener* /*ListenerHandle*/, | |
_In_ MsQuicListener* /*Listener*/, |
src/perf/lib/PerfServer.h
Outdated
@@ -90,14 +90,14 @@ class PerfServer : public PerfBase { | |||
|
|||
QUIC_STATUS | |||
ListenerCallback( | |||
_In_ HQUIC ListenerHandle, | |||
_In_ MsQuicListener* ListenerHandle, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's update ListenerHandle
to Listener
here and the others below.
src/test/lib/ApiTest.cpp
Outdated
QUIC_API | ||
DummyListenerCallback( | ||
T, | ||
void* Context, | ||
QUIC_LISTENER_EVENT* Event |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QUIC_API | |
DummyListenerCallback( | |
T, | |
void* Context, | |
QUIC_LISTENER_EVENT* Event | |
QUIC_API | |
DummyListenerCallback( | |
T, | |
void* Context, | |
QUIC_LISTENER_EVENT* Event |
Description
This PR completes the MsQuicListener implementation in that it adds a proper C++-style MsQuicListenerCallback type. This type is in line with other callback implementations in the C++ wrapper. Furthermore, the PR improves the consistency of some function signatures and adds a few retrieval functions for certain object parameters.
Testing
These changes are affect certain tests, which have been updated accordingly. New API additions where not added to the tests.
Documentation
Smaller API changes and additions have been made and might require doc updates.