Add missing static specifiers

[Maxim Dounin]

Hello!

On Tue, Feb 28, 2017 at 07:20:49PM +0000, Eran Kornblau wrote:

> Hi all,
> 
> Wrote a small script to find missing 'static's in my module -
> https://github.com/kaltura/nginx-vod-module/blob/master/test/test_static.py
> 
> I executed it on nginx core and found a few of those too, see attached patch.
> The logic for finding these was -
> 
> 1.       An exported symbol (found by running readelf on all object files)
> 
> 2.       Appears only in one source file (I could have checked the object imports instead, but anyway,
> that's how it works...)
> 
> 3.       Not mentioned in any header file (simple 'find whole words' search)
> 
> 4.       Does not end with '_module'
> 
> Btw, the script also complained about ngx_noaccepting and ngx_restart, I did not change those since
> all other flags there are exported.

Thank you, looks interesting.
Quick review below.

> # 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.


> 
> 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.

> @@ -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.

> 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.

> 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.

> 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.

[...]

-- 
Maxim Dounin
http://nginx.org/