[PATCH]Valgind: a complaint about uninitialized bytes in epoll_data_t

[Maxim Dounin]

Hello!

On Tue, Jul 02, 2013 at 02:00:40AM +0900, cubicdaiya wrote:

> Hello!
> 
> In Debian squeeze 32bit,
> Valgrind outputs the following message to nginx.
> 
> ==17124== Syscall param epoll_ctl(event) points to uninitialised byte(s)
> ==17124==    at 0x418F9CE: epoll_ctl (syscall-template.S:82)
> ==17124==    by 0x805FB35: ngx_event_process_init (ngx_event.c:853)

[...]

> The following patch eliminates this warning. Could you take a look at it?
> 
> # HG changeset patch
> # User Tatsuhiko Kubo <cubicdaiya at gmail.com>
> # Date 1372689447 -32400
> # Node ID cd8fd5cd74294554bb3777821e8703cf0fdf61d7
> # Parent  b66ec10e901a6fa0fc19937ceeb52b5ea1fbb706
> Valgrind: the complaint about uninitialized bytes in epoll_data_t.
> 
> diff -r b66ec10e901a -r cd8fd5cd7429 src/event/modules/ngx_epoll_module.c
> --- a/src/event/modules/ngx_epoll_module.c Fri Jun 28 13:55:05 2013 +0400
> +++ b/src/event/modules/ngx_epoll_module.c Mon Jul 01 23:37:27 2013 +0900
> @@ -417,6 +417,9 @@
>      }
> 
>      ee.events = events | (uint32_t) flags;
> +
> +    ngx_memzero(&ee.data, sizeof(epoll_data_t));
> +
>      ee.data.ptr = (void *) ((uintptr_t) c | ev->instance);
> 
>      ngx_log_debug3(NGX_LOG_DEBUG_EVENT, ev->log, 0,

I can't say I like the patch.

Calls to epoll_ctl() are at hot path, and doing an unneeded 
memzero to please Valgrind on some archs doesn't looks like a good 
idea.  Maybe under #if (NGX_VALGRIND) (and separately from normal 
assignments to ee structure; also not sure if we need memzero 
here, probably ee.data.u64 = 0 would be better).

Note well that the same coding pattern is used in many places 
across the epoll module, and changing only one place doesn't make 
sense.

-- 
Maxim Dounin
http://nginx.org/en/donation.html