Skip to content

Set LC_ALL=C before running ping process in Ping.Send() #50696

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

Merged
merged 2 commits into from
Apr 8, 2021

Conversation

smdn
Copy link
Contributor

@smdn smdn commented Apr 4, 2021

Fix #50363

smdn added 2 commits April 4, 2021 12:38
ping command output is affected by locale environment variables.
This will cause an exception in ParsePingUtilityOutput().
Set LC_ALL=C to get the same output regardless of the environment variables.

Fix dotnet#50363.
@ghost ghost added the area-System.Net label Apr 4, 2021
@ghost
Copy link

ghost commented Apr 4, 2021

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

Issue Details

Fix #50363

Author: smdn
Assignees: -
Labels:

area-System.Net

Milestone: -

Copy link
Contributor

@scalablecory scalablecory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me.

CC @wfurt for final approval

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Thanks for the fix @smdn

@@ -102,6 +102,7 @@
<Reference Include="System.Threading.ThreadPool" />
</ItemGroup>
<ItemGroup Condition="'$(TargetsUnix)' == 'true'">
<Reference Include="System.Collections.Specialized" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like. I had same doubt and build fails without it.

@wfurt wfurt merged commit d927fcb into dotnet:main Apr 8, 2021
@smdn
Copy link
Contributor Author

smdn commented Apr 8, 2021

Thank you for reviewing and merging!

@smdn smdn deleted the issue-50363 branch April 8, 2021 16:55
@ghost ghost locked as resolved and limited conversation to collaborators May 8, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ping.Send() throws PingException on Linux non-English locale
5 participants