-
Notifications
You must be signed in to change notification settings - Fork 316
Remove linq #1949
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
Remove linq #1949
Changes from 10 commits
ae4464e
b960371
5138ce2
06f2c2e
5508046
54d0500
887229c
ce10f20
2da5f36
47d344a
b3ddcb0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -5,7 +5,6 @@ | |||||
| using System; | ||||||
| using System.Collections.Generic; | ||||||
| using System.Diagnostics; | ||||||
| using System.Linq; | ||||||
| using System.Net; | ||||||
| using System.Net.Sockets; | ||||||
| using System.Text; | ||||||
|
|
@@ -192,52 +191,77 @@ private static byte[] SendUDPRequest(string browserHostname, int port, byte[] re | |||||
|
|
||||||
| IPAddress[] ipAddresses = SNICommon.GetDnsIpAddresses(browserHostname); | ||||||
| Debug.Assert(ipAddresses.Length > 0, "DNS should throw if zero addresses resolve"); | ||||||
|
|
||||||
| IPAddress[] ipv4Addresses = null; | ||||||
| IPAddress[] ipv6Addresses = null; | ||||||
| switch (ipPreference) | ||||||
| { | ||||||
| case SqlConnectionIPAddressPreference.IPv4First: | ||||||
| { | ||||||
| SsrpResult response4 = SendUDPRequest(ipAddresses.Where(i => i.AddressFamily == AddressFamily.InterNetwork).ToArray(), port, requestPacket, allIPsInParallel); | ||||||
| SplitIPv4AndIPv6(ipv4Addresses, out ipv4Addresses, out ipv6Addresses); | ||||||
|
||||||
| SplitIPv4AndIPv6(ipv4Addresses, out ipv4Addresses, out ipv6Addresses); | |
| SplitIPv4AndIPv6(ipAddresses, out ipv4Addresses, out ipv6Addresses); |
I'm assuming you're passing in the ipAddresses from line 192 instead of ipv4Addresses which is null on line 194; otherwise, it would always be Array.Empty<IPAddress>().
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.
Good catch. It is concerning that the CI didn't fail on this error.
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.
I agree. I was wondering about that as well. It turns out since this code is only run on Linux and requires sudo/administrative privileges to get the correct environment, there's currently no test coverage for this section unfortunately.
lcheunglci marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
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.
Maybe revert this condition and have fast exit? Code above looks a bit unnatural with this condition
ipv4Addresses = Array.Empty<IPAddress>();
ipv6Addresses = Array.Empty<IPAddress>();
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.
It's like this because they're out parameters. I suppose it's a personal style choice but I always assign the default values for out parameters at the start of the method so that anywhere inside the body I can easily return without worrying about whether they're assigned or not.
Uh oh!
There was an error while loading. Please reload this page.