add few builtins in runtime-light #1225
Conversation
|
|
||
| #pragma once | ||
|
|
||
| #include "runtime-common/core/allocator/runtime-allocator.h" |
There was a problem hiding this comment.
missing include <cstdint>
There was a problem hiding this comment.
- formatting
- braced initialization (highly recommended, but not required to get approve)
- replace
unlikely()with[[unlikely]] - I'd like to have better naming here, for example,
ctx->rt_ctx
| @@ -181,3 +181,16 @@ void f$setrawcookie(const string &name, const string &value, int64_t expire_or_o | |||
| } | |||
| kphp::http::header({static_SB_spare.c_str(), static_SB_spare.size()}, false, kphp::http::status::NO_STATUS); | |||
| } | |||
|
|
|||
| array<string> f$headers_list() noexcept { | |||
There was a problem hiding this comment.
Maybe move it's implementation into the header?
| @@ -181,3 +181,16 @@ void f$setrawcookie(const string &name, const string &value, int64_t expire_or_o | |||
| } | |||
| kphp::http::header({static_SB_spare.c_str(), static_SB_spare.size()}, false, kphp::http::status::NO_STATUS); | |||
| } | |||
|
|
|||
| array<string> f$headers_list() noexcept { | |||
| auto &http_server_instance_st{HttpServerInstanceState::get()}; | |||
There was a problem hiding this comment.
Just a thought: isn't it better to reduce number of variables in functions? Here, for example, we can get rid of http_server_instance_st as it's used only once
| auto &http_server_instance_st{HttpServerInstanceState::get()}; | ||
| const auto &headers{http_server_instance_st.headers()}; | ||
|
|
||
| array<string> list; |
There was a problem hiding this comment.
Let's add call to reserve
|
|
||
| array<string> list; | ||
| for (const auto & header : headers) { | ||
| string value{(header.first + ": " + header.second).c_str()}; |
There was a problem hiding this comment.
No need to temporary std::string here. It's better to use a combination of reserve_at_least and append
| k2::iconv(&res, cd, nullptr, nullptr, nullptr, nullptr); | ||
| } | ||
|
|
||
| k2::iconv_close(cd); |
There was a problem hiding this comment.
Let's have it wrapped with vk::final_action
|
|
||
| Optional<string> f$iconv(const string &input_encoding, const string &output_encoding, const string &input_str) noexcept { | ||
| iconv_t cd; | ||
| if (k2::iconv_open(&cd, output_encoding.c_str(), input_encoding.c_str()) != k2::errno_ok) { |
There was a problem hiding this comment.
Can you please use std::addressof instead of raw operator &.
Also, I'd appreciate if you add [[likely]]/[[unlikely]]. It's not a stopper btw
No description provided.