Skip to content
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

Add Span overload for Socket.ReceiveMessageFrom #43933

Closed
geoffkizer opened this issue Oct 28, 2020 · 3 comments · Fixed by #46285
Closed

Add Span overload for Socket.ReceiveMessageFrom #43933

geoffkizer opened this issue Oct 28, 2020 · 3 comments · Fixed by #46285
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Sockets help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@geoffkizer
Copy link
Contributor

We apparently missed this one in #938.

Proposed API

namespace System.Net.Sockets
{
    public partial class Socket : IDisposable
    {
        // existing: public int ReceiveMessageFrom(byte[] buffer, int offset, int size, ref System.Net.Sockets.SocketFlags socketFlags, ref System.Net.EndPoint remoteEP, out System.Net.Sockets.IPPacketInformation ipPacketInformation);

        public int ReceiveMessageFrom(Span<byte> buffer, ref System.Net.Sockets.SocketFlags socketFlags, ref System.Net.EndPoint remoteEP, out System.Net.Sockets.IPPacketInformation ipPacketInformation);
    }
@geoffkizer geoffkizer added area-System.Net.Sockets api-ready-for-review API is ready for review, it is NOT ready for implementation labels Oct 28, 2020
@geoffkizer geoffkizer added this to the 6.0.0 milestone Oct 28, 2020
@ghost
Copy link

ghost commented Oct 28, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Oct 28, 2020
@geoffkizer geoffkizer changed the title Add Span overloads for Socket.ReceiveMessageFrom Add Span overload for Socket.ReceiveMessageFrom Oct 28, 2020
@scalablecory scalablecory removed the untriaged New issue has not been triaged by the area owner label Oct 29, 2020
@karelz
Copy link
Member

karelz commented Oct 29, 2020

Triage: Should be simple to do once the API is formally reviewed.

@bartonjs
Copy link
Member

bartonjs commented Nov 10, 2020

Video

Looks good as proposed

namespace System.Net.Sockets
{
    public partial class Socket : IDisposable
    {
        public int ReceiveMessageFrom(Span<byte> buffer, ref System.Net.Sockets.SocketFlags socketFlags, ref System.Net.EndPoint remoteEP, out System.Net.Sockets.IPPacketInformation ipPacketInformation);
    }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Nov 10, 2020
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Dec 21, 2020
ovebastiansen added a commit to ovebastiansen/runtime that referenced this issue Jan 29, 2021
antonfirsov pushed a commit that referenced this issue Feb 2, 2021
* adding span version of ReceiveMessageFrom

fix #43933

* Started using new testsetup for the receivemessagefrom

* removed code duplication in socketpal.windows

* removed unused buffers parameter

* adding non-zero offset to the ReceiveMessageFrom test

* added documentation

* offset fix for EAP tests

* fixing pr comments
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 2, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Mar 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Sockets help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants