Skip to content

Use Nginx core function in ngx_http_lua_rebase_path #835

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Use Nginx core function in ngx_http_lua_rebase_path #835

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a15f085
Switched to NGINX core's ngx_get_full_name
Someguynamedpie Aug 9, 2016
15feb11
Typos and fixing some of the very picky warnings.
Someguynamedpie Aug 9, 2016
4f5e59e
Mixed up function synax
Someguynamedpie Aug 9, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 18 additions & 29 deletions src/ngx_http_lua_util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,40 +371,29 @@ ngx_http_lua_del_thread(ngx_http_request_t *r, lua_State *L,
u_char *
ngx_http_lua_rebase_path(ngx_pool_t *pool, u_char *src, size_t len)
{
u_char *p, *dst;

if (len == 0) {
ngx_str_t nsrc;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nsrc is not a good name since the n prefix reminds me of number. But it's actually a string.

u_char *optr;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style: identifiers of local variable declarations must be aligned up (see other similar places in the existing code base).


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra spaces in this empty line (and also all the empty lines below).

if (len == 0)
return NULL;
}

if (src[0] == '/') {
/* being an absolute path already */
dst = ngx_palloc(pool, len + 1);
if (dst == NULL) {
return NULL;
}

p = ngx_copy(dst, src, len);

*p = '\0';

return dst;
}

dst = ngx_palloc(pool, ngx_cycle->prefix.len + len + 1);
if (dst == NULL) {

nsrc.len = len;
nsrc.data = ngx_palloc(pool, len+1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ngx_palloc may return NULL and we should handle this case.


if (!nsrc.data)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style: we always require curly braces for statement's code blocks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think testing against NULL here is better.

return NULL;
}

p = ngx_copy(dst, ngx_cycle->prefix.data, ngx_cycle->prefix.len);
p = ngx_copy(p, src, len);

*p = '\0';

return dst;

optr = ngx_copy(nsrc.data, src, len);
*optr = '\0';

if(ngx_get_full_name(pool, (ngx_str_t*)&ngx_cycle->prefix, &nsrc))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a space after if, and ngx_str_t, and before &ngx_cycle.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also need curly braces for the if statement body.

return NULL;
return nsrc.data;
}



Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the extra blank line here?

ngx_int_t
ngx_http_lua_send_header_if_needed(ngx_http_request_t *r,
ngx_http_lua_ctx_t *ctx)
Expand Down