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 <[email protected]> > # 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 _______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
