Add missing static specifiers

Eran Kornblau eran.kornblau at kaltura.com
Tue Feb 28 21:20:37 UTC 2017


Thanks for the review! 
Updated patch attached, comments inline below

> Hello!
> 
> > # HG changeset patch
> > # User Eran Kornblau <erankor at gmail.com> # Date 1488300547 18000
> > #      Tue Feb 28 11:49:07 2017 -0500
> > # Node ID 4b4b8f5413a4a1679d6ad0aa444e29e3f55b6b2a
> > # Parent  8b7fd958c59f8280d167fe7dd93f1942bfed5876
> > add missing static specifiers
> 
> Style nitpicking:
> 
> Added missing static specifiers.
> 
Changed

> 
> > 
> > diff -r 8b7fd958c59f -r 4b4b8f5413a4 src/core/ngx_resolver.c
> > --- a/src/core/ngx_resolver.c	Mon Feb 27 22:36:15 2017 +0300
> > +++ b/src/core/ngx_resolver.c	Tue Feb 28 11:49:07 2017 -0500
> > @@ -56,7 +56,7 @@
> >          ((u_char *) (n) - offsetof(ngx_resolver_node_t, node))
> >  
> >  
> > -ngx_int_t ngx_udp_connect(ngx_resolver_connection_t *rec);
> > +static ngx_int_t ngx_udp_connect(ngx_resolver_connection_t *rec);
> >  ngx_int_t ngx_tcp_connect(ngx_resolver_connection_t *rec);
> >  
> >  
> 
> I'm curious why ngx_udp_connect() was catched, but exactly identical ngx_tcp_connect() wasn't.
> 
Good catch! The reason is that while I applied the changes against master,
I'm usually working on old version (1.8.0) and I executed the script against 
that one, this function did not exist then.
Anyway, I ran the script now against master and it did find this function, 
as well as a few additional stat variables which I added now as well.


> > @@ -4379,7 +4379,7 @@
> >  }
> >  
> >  
> > -ngx_int_t
> > +static ngx_int_t
> >  ngx_udp_connect(ngx_resolver_connection_t *rec)  {
> >      int                rc;
> > diff -r 8b7fd958c59f -r 4b4b8f5413a4 src/event/modules/ngx_epoll_module.c
> > --- a/src/event/modules/ngx_epoll_module.c	Mon Feb 27 22:36:15 2017 +0300
> > +++ b/src/event/modules/ngx_epoll_module.c	Tue Feb 28 11:49:07 2017 -0500
> > @@ -176,7 +176,7 @@
> >  };
> >  
> >  
> > -ngx_event_module_t  ngx_epoll_module_ctx = {
> > +static ngx_event_module_t  ngx_epoll_module_ctx = {
> >      &epoll_name,
> >      ngx_epoll_create_conf,               /* create configuration */
> >      ngx_epoll_init_conf,                 /* init configuration */
> > diff -r 8b7fd958c59f -r 4b4b8f5413a4 src/event/ngx_event.c
> > --- a/src/event/ngx_event.c	Mon Feb 27 22:36:15 2017 +0300
> > +++ b/src/event/ngx_event.c	Tue Feb 28 11:49:07 2017 -0500
> > @@ -165,7 +165,7 @@
> >  };
> >  
> >  
> > -ngx_event_module_t  ngx_event_core_module_ctx = {
> > +static ngx_event_module_t  ngx_event_core_module_ctx = {
> >      &event_core_name,
> >      ngx_event_core_create_conf,            /* create configuration */
> >      ngx_event_core_init_conf,              /* init configuration */
> 
> Note that all event modules currently use non-static contexts.  
> Obviously your script can't catch symbols in modules you don't have compiled in, but a simple grep will allow to properly extend this to other modules.
> 
Added

> > diff -r 8b7fd958c59f -r 4b4b8f5413a4 src/http/modules/ngx_http_charset_filter_module.c
> > --- a/src/http/modules/ngx_http_charset_filter_module.c	Mon Feb 27 22:36:15 2017 +0300
> > +++ b/src/http/modules/ngx_http_charset_filter_module.c	Tue Feb 28 11:49:07 2017 -0500
> > @@ -123,7 +123,7 @@
> >  static ngx_int_t ngx_http_charset_postconfiguration(ngx_conf_t *cf);
> >  
> >  
> > -ngx_str_t  ngx_http_charset_default_types[] = {
> > +static ngx_str_t  ngx_http_charset_default_types[] = {
> >      ngx_string("text/html"),
> >      ngx_string("text/xml"),
> >      ngx_string("text/plain"),
> 
> The ngx_http_xslt_default_types variable seems to be exactly identical.
> 
Added

> > diff -r 8b7fd958c59f -r 4b4b8f5413a4 src/http/modules/ngx_http_static_module.c
> > --- a/src/http/modules/ngx_http_static_module.c	Mon Feb 27 22:36:15 2017 +0300
> > +++ b/src/http/modules/ngx_http_static_module.c	Tue Feb 28 11:49:07 2017 -0500
> > @@ -14,7 +14,7 @@
> >  static ngx_int_t ngx_http_static_init(ngx_conf_t *cf);
> >  
> >  
> > -ngx_http_module_t  ngx_http_static_module_ctx = {
> > +static ngx_http_module_t  ngx_http_static_module_ctx = {
> >      NULL,                                  /* preconfiguration */
> >      ngx_http_static_init,                  /* postconfiguration */
> >  
> 
> As per
> 
> $ grep -rh ngx_http_module_t src/ | grep -v '^static'
> 
> there is also ngx_http_gzip_static_module_ctx which is also not static.
> 
Added

> > diff -r 8b7fd958c59f -r 4b4b8f5413a4 src/http/ngx_http_upstream.c
> > --- a/src/http/ngx_http_upstream.c	Mon Feb 27 22:36:15 2017 +0300
> > +++ b/src/http/ngx_http_upstream.c	Tue Feb 28 11:49:07 2017 -0500
> > @@ -188,7 +188,7 @@
> >  #endif
> >  
> >  
> > -ngx_http_upstream_header_t  ngx_http_upstream_headers_in[] = {
> > +static ngx_http_upstream_header_t  ngx_http_upstream_headers_in[] = {
> >  
> >      { ngx_string("Status"),
> >                   ngx_http_upstream_process_header_line,
> > diff -r 8b7fd958c59f -r 4b4b8f5413a4 src/os/unix/ngx_linux_init.c
> > --- a/src/os/unix/ngx_linux_init.c	Mon Feb 27 22:36:15 2017 +0300
> > +++ b/src/os/unix/ngx_linux_init.c	Tue Feb 28 11:49:07 2017 -0500
> > @@ -9,8 +9,8 @@
> >  #include <ngx_core.h>
> >  
> >  
> > -u_char  ngx_linux_kern_ostype[50];
> > -u_char  ngx_linux_kern_osrelease[50];
> > +static u_char  ngx_linux_kern_ostype[50]; static u_char  
> > +ngx_linux_kern_osrelease[50];
> 
> There are various OS-specific variables for various other platforms as well.  It would be a good idea to either review them all, or left them as is.
> 
Only one I could find is SERVICE_TABLE_ENTRY st (went over win32 files manually)
Added it

> [...]
> 
> --
> Maxim Dounin
> http://nginx.org/
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>

Eran
-------------- next part --------------
A non-text attachment was scrubbed...
Name: nginx-add-static2.patch
Type: application/octet-stream
Size: 10239 bytes
Desc: nginx-add-static2.patch
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20170228/5edcb27e/attachment-0001.obj>


More information about the nginx-devel mailing list