feature: exposed ngx_http_lua_ffi_str_t for other Nginx modules.#1273
feature: exposed ngx_http_lua_ffi_str_t for other Nginx modules.#1273spacewander wants to merge 1 commit into
Conversation
| #include <lua.h> | ||
| #include <stdint.h> | ||
|
|
||
| #include "ngx_http_lua_api_common.h" |
There was a problem hiding this comment.
It's wrong to include common.h in api.h since it exposes all ngx_lua's internal types and symbols, which is not desired.
There was a problem hiding this comment.
@agentzh
No, ngx_http_lua_api_common.h is not ngx_http_lua_common.h.
There was a problem hiding this comment.
@spacewander Why introduce this new common.h file? It's confusing ;) Let's simply put everything inside this api.h. This file is small anyway.
There was a problem hiding this comment.
@agentzh
The ngx_http_lua_ffi_str_t is required inside of the lua-nginx-module too. Put it into api.h will require the other headers to include the public API header. I think it could be more flexible by adding a middle layer.
There was a problem hiding this comment.
@spacewander Hmm, I disagree here. Internally we should use ngx_str_t directly.
There was a problem hiding this comment.
@agentzh
ngx_http_lua_ffi_str_t is used in ngx_http_lua_util.h:
https://github.com/openresty/lua-nginx-module/blob/master/src/ngx_http_lua_util.h#L25
If we put ngx_http_lua_ffi_str_t into api.h, then we should include the public API header into the internal utility header, which is strange.
There was a problem hiding this comment.
@spacewander I don't want that api_common.h file. It's horrible and confusing. You can include the whole api.h file if necessary. I don't mind.
| #include <lua.h> | ||
| #include <stdint.h> | ||
|
|
||
| #include "ngx_http_lua_api_common.h" |
There was a problem hiding this comment.
@spacewander Why introduce this new common.h file? It's confusing ;) Let's simply put everything inside this api.h. This file is small anyway.
|
@spacewander Committed 809192c instead. Thanks! |
2 participants
I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.