-
Notifications
You must be signed in to change notification settings - Fork 198
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
lua_add_variable - initialize value (add optional arguments) #107
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1209,12 +1209,29 @@ ngx_stream_lua_undefined_var(ngx_stream_session_t *s, | |
} | ||
|
||
|
||
static ngx_int_t | ||
ngx_stream_lua_var(ngx_stream_session_t *s, | ||
ngx_stream_variable_value_t *v, uintptr_t data) | ||
{ | ||
ngx_str_t *str = (ngx_str_t *) data; | ||
|
||
v->not_found = 0; | ||
v->valid = 1; | ||
v->no_cacheable = 0; | ||
v->not_found = 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two |
||
v->len = str->len; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can just write There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can not, v is type ngx_variable_value_t, str is ngx_str_t |
||
v->data = str->data; | ||
|
||
return NGX_OK; | ||
} | ||
|
||
|
||
char * | ||
ngx_stream_lua_add_variable(ngx_conf_t *cf, ngx_command_t *cmd, | ||
void *conf) | ||
{ | ||
ngx_stream_variable_t *var; | ||
ngx_str_t *value; | ||
ngx_str_t *value, *data; | ||
ngx_int_t ret; | ||
|
||
value = cf->args->elts; | ||
|
@@ -1234,8 +1251,32 @@ ngx_stream_lua_add_variable(ngx_conf_t *cf, ngx_command_t *cmd, | |
return NGX_CONF_ERROR; | ||
} | ||
|
||
if (var->get_handler == NULL) { | ||
var->get_handler = ngx_stream_lua_undefined_var; | ||
if (cf->args->nelts == 2) { | ||
if (var->get_handler == NULL) { | ||
var->get_handler = ngx_stream_lua_undefined_var; | ||
} | ||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: needs a blank line before this line. |
||
if (var->get_handler == NULL) { | ||
var->get_handler = ngx_stream_lua_var; | ||
} | ||
|
||
if (var->data) { | ||
data = (ngx_str_t *) var->data; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this code path is reacheable. Will you explain? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is called when the variable is redefined. |
||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto. |
||
data = ngx_pnalloc(cf->pool, sizeof(ngx_str_t)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should use |
||
if (data == NULL) { | ||
return NGX_CONF_ERROR; | ||
} | ||
} | ||
data->len = value[2].len; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: need a newline before this. |
||
|
||
data->data = ngx_pnalloc(cf->pool, value[2].len); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto. Also, I wonder if the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes this can be saved. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right |
||
if (data->data == NULL) { | ||
return NGX_CONF_ERROR; | ||
} | ||
ngx_memcpy(data->data, value[2].data, value[2].len); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: need a newline before this. |
||
|
||
var->data = (uintptr_t) data; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This type casting looks weird. I don't think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, my bad, it's indeed declared as |
||
} | ||
|
||
ret = ngx_stream_get_variable_index(cf, value + 1); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function name is confusing. I think it should be
ngx_stream_lua_var_default_value
instead.Also, better declare this at the beginning of this
.c
file.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not better that the declarations under ngx_stream_lua_undefined_var ?