Porting System.Net.Security#3113
Conversation
There was a problem hiding this comment.
Why is the StreamAsyncHelper class buried in this dll? Why not put into src/Common somewhere? Then it could be reused by others.
|
LGTM |
There was a problem hiding this comment.
Have these files been formatted?
There was a problem hiding this comment.
To expand a bit, I think that this line should be
if (!_stream.CanRead)
{
throw __Error.GetReadNotSupported();
}
There are a number of other instances as well.
There was a problem hiding this comment.
Good catch: this file got deleted during work I did for this PR. This is just bringing it back to the same location.
It appears that the file never got formatted for some reason. I'll run the tools again.
There was a problem hiding this comment.
Not handled by CodeFormatter. Will manually change.
Tracking with dotnet/codeformatter#177.
|
LGTM aside from some code style issues. |
There was a problem hiding this comment.
Is this comment still relevant?
There was a problem hiding this comment.
The comment is from the original code. I'd keep it for now since it uncovers an issue in marshaling the struct from a native buffer: There is an assumption made that padding won't be added on any any architecture.
Changed the comment to reflect this and linked to #3114.
10 participants
Missing issues for TODO comments. Will create the tracking issue and update the code, author (dotnetbot) and reference the tracking issue.
@davidsh @bartonjs @pgavlin @stephentoub @SidharthNabar PTAL.