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

use String instead of RequestUri for HTTP path #10

Closed
wants to merge 1 commit into from
Closed

use String instead of RequestUri for HTTP path #10

wants to merge 1 commit into from

Conversation

pyfisch
Copy link
Contributor

@pyfisch pyfisch commented Jan 24, 2016

Addresses #8. Do not merge yet.

You said the path could be a slice of a buffer, but using &str with the current code does not work since the Transport in parse_headers is borrowed too long mutably by Head. I think it is possible to use slices but parse_headers needs to be rewritten. Maybe it would be easier if the headers weren't passed to request_start again?

@tailhook
Copy link
Owner

Hm, the idea was that Head is passed by reference to the headers_received, because rotor still needs some info from headers. Then it passes ownership to the application in request_start.

Perhaps it's possible to extract everything needed before executing headers_received (if it doesn't require to clone/allocate memory). Then we can get rid of request_start entirely.

@pyfisch
Copy link
Contributor Author

pyfisch commented Jan 24, 2016

  • fn headers_received(head: &Head, scope: &mut Scope<Self::Context>) -> Result<(Self, RecvMode, Deadline), StatusCode>;
  • fn request_start(self, head: Head, response: &mut Response, scope: &mut Scope<Self::Context>) -> Option<Self>;

The methods differ in their return type and that request_start expects a response object. If request_start is removed there is no way to construct the response immediatly after the headers were received or should the headers_received method be extended to accept a response object?

@tailhook
Copy link
Owner

or should the headers_received method be extended to accept a response object?

Yes, exactly. (or at least I don't see any difficulties with that at the moment)

@pyfisch
Copy link
Contributor Author

pyfisch commented Jan 24, 2016

There may be a slight increase in memory allocation because the Response is also created for failing requests.

@tailhook
Copy link
Owner

There may be a slight increase in memory allocation because the Response is also created for failing requests.

No, Response does not have heap allocated parts.

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.

None yet

2 participants