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

misc(*) remove from ngx_wasm_lua_resolver_resolve call freeing 'rslv_ctx' #624

Closed
wants to merge 1 commit into from

Conversation

casimiro
Copy link
Contributor

@casimiro casimiro commented Nov 15, 2024

Presently the behavior of ngx_wasm_lua_resolver_resolve mimics the one from ngx_resolve_name, which frees its argument 'rslv_ctx' in case of any errors.

This commit changes ngx_wasm_lua_resolver_resolve to not free 'rslv_ctx' if any errors occur, delegating the responsibility to its caller.

Although it makes ngx_wasm_lua_resolver_resolve less consistent with ngx_resolve_name, this change allows the upcoming logic exposing ngx_wasm_lua_resolver_resolve as a PW foreign-function to allocate 'rslv_ctx' using memory from the pwexec's pool, instead of from malloc.

Copy link

codecov bot commented Nov 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.83289%. Comparing base (9238d4a) to head (e5d249d).

Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##                main        #624         +/-   ##
===================================================
- Coverage   90.84180%   90.83289%   -0.00892%     
===================================================
  Files             52          52                 
  Lines          11214       11214                 
===================================================
- Hits           10187       10186          -1     
- Misses          1027        1028          +1     
Files with missing lines Coverage Δ
src/common/lua/ngx_wasm_lua_resolver.c 75.82418% <ø> (-0.51991%) ⬇️
src/common/ngx_wasm_socket_tcp.c 89.53804% <100.00000%> (+0.02849%) ⬆️

... and 1 file with indirect coverage changes

Flag Coverage Δ
unit 90.58118% <100.00000%> (+0.00095%) ⬆️
valgrind 82.43654% <100.00000%> (-0.07204%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

…v_ctx'

Presently the behavior of `ngx_wasm_lua_resolver_resolve` mimics the one
from ngx_resolve_name, which frees its argument 'rslv_ctx' in case of
any errors.

This commit changes `ngx_wasm_lua_resolver_resolve` to not free 'rslv_ctx'
if any errors occur, delegating the responsibility to its caller.

Although it makes `ngx_wasm_lua_resolver_resolve` less consistent with
ngx_resolve_name, this change allows the upcoming logic exposing
`ngx_wasm_lua_resolver_resolve` as a PW foreign-function to allocate
'rslv_ctx' using memory from the pwexec's pool, instead of from malloc.
@casimiro casimiro closed this Nov 15, 2024
@thibaultcha thibaultcha deleted the misc/lua_resolver branch November 15, 2024 17:59
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.

1 participant