GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Expose OS-chosen address and port; force IPv4 by eile · Pull Request #707 · cpp-netlib/cpp-netlib

Conversation

eile commented Nov 10, 2016

Copy link

Copy Markdown

No description provided.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Expose OS-chosen address and port; force IPv4

b886faa

deanberris reviewed

Nov 14, 2016

View reviewed changes

deanberris left a comment

Copy link

Copy Markdown

Member

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.

Can you explain why this is necessary, and whether it's possible for you to make these options instead of forcing the defaults?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

CMakeLists.txt

		option( CPP-NETLIB_BUILD_SHARED_LIBS "Build cpp-netlib as shared libraries." ON )
		option( CPP-NETLIB_BUILD_TESTS "Build the cpp-netlib project tests." OFF)
		option( CPP-NETLIB_BUILD_EXPERIMENTS "Build the cpp-netlib project experiments." OFF)
		option( CPP-NETLIB_BUILD_EXAMPLES "Build the cpp-netlib project examples." OFF)

deanberris Nov 14, 2016

Copy link

Copy Markdown

Member

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'm not comfortable with changing these defaults. Is there a reason why you don't set these from your project?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

anonimal reacted with thumbs up emoji

All reactions

👍 1 reaction

Comment thread

boost/network/protocol/http/server/sync_server.hpp

		system::error_code error;
		tcp::resolver resolver(service_);
		tcp::resolver::query query(address_, port_);
		tcp::resolver::query query(tcp::v4(), address_, port_);

deanberris Nov 14, 2016

Copy link

Copy Markdown

Member

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.

There seems to be no reason for this -- can you not make this explicit in your code instead? As part of the options?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Fix MSVC build

62269d2

deanberris commented Dec 15, 2016

Copy link

Copy Markdown

Member

Hi @eile -- I sent some comments a while back and I'm unclear as to why this is necessary and why this isn't an optional thing (as opposed to something that forces IPv4). Do you mind making it so that we can configure what kind of IP addresses we'd like to get from the DNS would be? Then I can go ahead and merge the updated version.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

eile commented Dec 15, 2016

Copy link

Copy Markdown

Author

Hi @deanberris: @tribal-tec and me where a bit swamped lately, we'll get back to you on this.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

deanberris commented Dec 15, 2016

Copy link

Copy Markdown

Member

All good, thanks @eile -- no rush. :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

deanberris commented Dec 15, 2016

Copy link

Copy Markdown

Member

Also by the way, I just noticed that you're attempting to make a change to the 0.11-devel branch, which we aren't actively maintaining anymore. We're trying to get everyone to use the 0.13-release branch instead. I'm happy to still merge an updated version of this in 0.11-devel, but I'm not really up for having another release on the 0.11.x line.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Use system includes for boost

ed0b1b6

dnachbaur mentioned this pull request

Jan 24, 2017

git_external for ccp-netlib HBPVIS/ZeroEQ#196

Closed

dnachbaur mentioned this pull request

Feb 1, 2017

Expose OS-chosen address and port from async_server #729

Merged

dnachbaur commented Feb 1, 2017

Copy link

Copy Markdown

Please close this in favor of #729

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

eile closed this

Feb 6, 2017

Reviewers

deanberris

deanberris left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.