From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [PATCH] eal_interrupts.c: properly init struct epoll_event (valgrind) Date: Sun, 14 Feb 2016 12:22:09 -0800 Message-ID: <20160214122209.38b0efb6@xeon-e3> References: <1455345678-17662-1-git-send-email-mhall@mhcomputing.net> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org To: Matthew Hall Return-path: Received: from mail-pa0-f41.google.com (mail-pa0-f41.google.com [209.85.220.41]) by dpdk.org (Postfix) with ESMTP id EF0797E6A for ; Sun, 14 Feb 2016 21:21:56 +0100 (CET) Received: by mail-pa0-f41.google.com with SMTP id yy13so74176737pab.3 for ; Sun, 14 Feb 2016 12:21:56 -0800 (PST) In-Reply-To: <1455345678-17662-1-git-send-email-mhall@mhcomputing.net> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Fri, 12 Feb 2016 22:41:18 -0800 Matthew Hall wrote: > Signed-off-by: Matthew Hall > --- > lib/librte_eal/linuxapp/eal/eal_interrupts.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c > index 06b26a9..b33ccdb 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c > +++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c > @@ -800,6 +800,7 @@ static __attribute__((noreturn)) void * > eal_intr_thread_main(__rte_unused void *arg) > { > struct epoll_event ev; > + memset(&ev, 0, sizeof(ev)); > > /* host thread, never break out */ > for (;;) { I wonder why valgrind is giving this false report. The only place this data structure is used is here, and all fields in epoll_event are correctly set. TAILQ_FOREACH(src, &intr_sources, next) { if (src->callbacks.tqh_first == NULL) continue; /* skip those with no callbacks */ ev.events = EPOLLIN | EPOLLPRI; ev.data.fd = src->intr_handle.fd; /** * add all the uio device file descriptor * into wait list. */ if (epoll_ctl(pfd, EPOLL_CTL_ADD, src->intr_handle.fd, &ev) < 0){ rte_panic("Error adding fd %d epoll_ctl, %s\n", src->intr_handle.fd, strerror(errno)); A better patch would be to move the data structure into the code block used, and get rid of the useless else (rte_panic never returns); and fix the indentation, and use C99 initialization which should make valgrind happier. The moral is don't just slap memsets around diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c index 06b26a9..d53826e 100644 --- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c +++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c @@ -799,8 +799,6 @@ eal_intr_handle_interrupts(int pfd, unsigned totalfds) static __attribute__((noreturn)) void * eal_intr_thread_main(__rte_unused void *arg) { - struct epoll_event ev; - /* host thread, never break out */ for (;;) { /* build up the epoll fd with all descriptors we are to @@ -834,20 +832,22 @@ eal_intr_thread_main(__rte_unused void *arg) TAILQ_FOREACH(src, &intr_sources, next) { if (src->callbacks.tqh_first == NULL) continue; /* skip those with no callbacks */ - ev.events = EPOLLIN | EPOLLPRI; - ev.data.fd = src->intr_handle.fd; + + struct epoll_event ev = { + .events = EPOLLIN | EPOLLPRI, + .data.fd = src->intr_handle.fd, + }; /** * add all the uio device file descriptor * into wait list. */ if (epoll_ctl(pfd, EPOLL_CTL_ADD, - src->intr_handle.fd, &ev) < 0){ + src->intr_handle.fd, &ev) < 0) rte_panic("Error adding fd %d epoll_ctl, %s\n", src->intr_handle.fd, strerror(errno)); - } - else - numfds++; + + numfds++; } rte_spinlock_unlock(&intr_lock); /* serve the interrupt */