Hello to all,
On Wed, 01 Sep 2010 18:28:12 -1000
Derrick Petzold <contact@derrickpetzold.com> wrote:
> This patch adds support for ignoring files in the auto index. Its
> modeled somewhat after Apache's IndexIgnore
>
> [...]
>
> Questions and comments are welcome.
I hope you don't mind I picked ideas from it to add this feature to my
fancyindex module [1] :-)
I have a couple of comments -- none of them important.
> +#if (NGX_PCRE)
> +
> + {
> + ngx_str_t str;
> + str.data = ngx_de_name(&dir);
> + str.len = len;
I would write the initialization of “str” like this:
ngx_str_t str = { len, ngx_de_name(&dir) };
But that's just a matter of preference ;)
> +#else
> + {
> + u_int found_match = 0;
> + ngx_str_t *s;
> + if (alcf->ignore) {
> + s = alcf->ignore->elts;
> + for (i = 0; i < alcf->ignore->nelts; i++) {
> + if (ngx_strcmp(ngx_de_name(&dir), s[i].data) == 0) {
> + found_match = 1;
> + break;
> + }
> + }
> + if (found_match) {
> + continue;
> + }
> + }
> + }
> +#endif
In this particular case I would declare ”found_match” and “*s” inside
the “if” block, to avoid having the dangling braces, like this:
#else
if (alcf->ignore) {
u_int found_match = 0;
ngx_str_t *s = alcf->ignore->elts;
/* ... */
}
#endif
The net effect is the same, and it looks easier on the eyes, IMHO.
By the way, very nice patch.
Cheers,
---
[1] http://gitorious.org/ngx-fancyindex
--
-Adrian
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://nginx.org/mailman/listinfo/nginx-devel