Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Add shim for libcurl version info.#4186

Merged
eerhardt merged 1 commit into
dotnet:masterfrom
eerhardt:ShimLibCurl
Oct 28, 2015
Merged

Add shim for libcurl version info.#4186
eerhardt merged 1 commit into
dotnet:masterfrom
eerhardt:ShimLibCurl

Conversation

@eerhardt
Copy link
Copy Markdown
Member

Adding a native shim for curl_version_info and removing the managed constants that are no longer needed.

@kapilash @nguerrera @sokket @stephentoub

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: I wonder if this should be "curl_version_info" rather than "GetCurlVersionInfo", since curl_version_info is part of the libcurl public interface whereas GetCurlVersionInfo is an internal implementation detail of our solution.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Changed it to curl_version_info.

@stephentoub
Copy link
Copy Markdown
Member

One minor wording question, otherwise looks great.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It looks like we call this from one place and we pass in each variable...why not make these asserts that they are non-nullptr and then just set them?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was always taught to check pointers before dereferencing them in C++ code.

@jonmill
Copy link
Copy Markdown

jonmill commented Oct 28, 2015

One minor question but otherwise LGTM

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Other projects depend on this file and is the cause of build failure.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed.

Adding a native shim for curl_version_info and removing the managed constants that are no longer needed.
eerhardt added a commit that referenced this pull request Oct 28, 2015
Add shim for libcurl version info.
@eerhardt eerhardt merged commit 8924a5c into dotnet:master Oct 28, 2015
@eerhardt eerhardt deleted the ShimLibCurl branch October 28, 2015 15:03
@karelz karelz modified the milestone: 1.0.0-rtm Dec 3, 2016
@karelz karelz added the os-linux Linux OS (any supported distro) label Mar 8, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Add shim for libcurl version info.

Commit migrated from dotnet/corefx@8924a5c
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Net os-linux Linux OS (any supported distro)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants