Skip to content
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

Update uri.cpp #93

Closed
wants to merge 1 commit into from
Closed

Update uri.cpp #93

wants to merge 1 commit into from

Conversation

TheAquaMan
Copy link
Contributor

Fix for uri::swap method

Fix for uri::swap method
@glynos
Copy link
Member

glynos commented Dec 4, 2016

Thanks for the PR. But what does this fix? Can you add a test?

@TheAquaMan
Copy link
Contributor Author

TheAquaMan commented Dec 4, 2016

Without proposed changes uri::swap method worked in incorrect way: this method swapped uri_ and uri_view_, but uri_parts_ members were not swapped, this method used other.uri_parts_ for both uri objects. This method overwrote this->uri_parts_ by other.uri_parts_ and then wrote to this->uri_parts_ to other.uri_parts_, so both uris had the same other.uri_parts_ value.
As result wrong return values for host(), port(), query(), etc, methods for one of swapped objects

@glynos
Copy link
Member

glynos commented Dec 16, 2016

Sorry it has taken me a while to respond. Your changes look correct, but I'd still like to see a unit test before merging this PR.

@glynos
Copy link
Member

glynos commented May 26, 2017

@TheAquaMan I applied this fix in PR #99, including a test. Can you confirm if that works for you?

@glynos
Copy link
Member

glynos commented May 26, 2017

Fix was applied on master (see PR #99)

@glynos glynos closed this May 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants