Skip to content

content_length type comparison error #753

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 1 commit into from
Apr 7, 2017

Conversation

ekigwana
Copy link

@ekigwana ekigwana commented Apr 6, 2017

Type was std::size_t and was changed to long long in commit f8ff77d. It is also initialized to
-1 in parse_headers_real(...) to help determine parse error.

Signed-off-by: Edward Kigwana ekigwana@gmail.com

commit f8ff77d. It is also initialized to
-1 in parse_headers_real(...).

Signed-off-by: Edward Kigwana <ekigwana@gmail.com>
@deanberris deanberris self-requested a review April 6, 2017 23:09
Copy link
Member

@deanberris deanberris left a comment

Choose a reason for hiding this comment

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

Thanks @ekigwana -- it's not a blocker, but have you thought about how hard it would be if we used an optional<size_t> instead for the content length? That way we don't need to do funny things with casting the value.

If that's going to take longer or more effort then I'm fine with this in the meantime.

@ekigwana
Copy link
Author

ekigwana commented Apr 7, 2017

I have not looked at it but it seems easy enough to do. I'll need a few days. Put this request on hold for a bit please.

@deanberris
Copy link
Member

LGTM

That's fine to do after, I'm happy to merge this in the meantime. :)

@deanberris deanberris merged commit fa76515 into cpp-netlib:master Apr 7, 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