Here is some misc that I fixed while dealing with other bugs and writing more test cases.
Created attachment 112837 [details] [review] [PATCH 1/4] socket_set_epoll_add: initialize all bytes of struct epoll_event
Created attachment 112838 [details] [review] [PATCH 2/4] bus: exit on fatal errors even if not syslogging
Created attachment 112839 [details] [review] [PATCH 3/4] lcov: use builddir, not srcdir It seems lcov (or gcc?) has changed its paths since last time this worked.
Created attachment 112840 [details] [review] [PATCH 4/4] Enable subdir-objects Automake option It has been supported since at least 1.10, and its absence is deprecated since 1.14.
Comment on attachment 112837 [details] [review] [PATCH 1/4] socket_set_epoll_add: initialize all bytes of struct epoll_event Review of attachment 112837 [details] [review]: ----------------------------------------------------------------- What about socket_set_epoll_enable() and socket_set_epoll_disable()? Probably also socket_set_epoll_remove() since it only initialises the first member of the struct epoll_event. Also main().
Comment on attachment 112838 [details] [review] [PATCH 2/4] bus: exit on fatal errors even if not syslogging Review of attachment 112838 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 112839 [details] [review] [PATCH 3/4] lcov: use builddir, not srcdir Review of attachment 112839 [details] [review]: ----------------------------------------------------------------- No idea what’s changed, but ++ from me if this fixes the build. Although perhaps we should bump the LCOV dependency? Or, in fact, add one to configure.ac in an AX_CHECK_TOOL() macro? See what AX_CODE_COVERAGE does (perhaps we should be using that instead, for consistency?): http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=blob_plain;f=m4/ax_code_coverage.m4
Comment on attachment 112840 [details] [review] [PATCH 4/4] Enable subdir-objects Automake option Review of attachment 112840 [details] [review]: ----------------------------------------------------------------- ++
(In reply to Philip Withnall from comment #5) > What about socket_set_epoll_enable() and socket_set_epoll_disable()? > Probably also socket_set_epoll_remove() since it only initialises the first > member of the struct epoll_event. Also main(). A fair point. The test I was looking at didn't seem to exercise those.
(In reply to Philip Withnall from comment #7) > No idea what’s changed, but ++ from me if this fixes the build. Although > perhaps we should bump the LCOV dependency? Or, in fact, add one to > configure.ac in an AX_CHECK_TOOL() macro? See what AX_CODE_COVERAGE does > (perhaps we should be using that instead, for consistency?) I'd rather land the obvious fix, then switch to AX_CODE_COVERAGE some other time. Bug #88922.
Comment on attachment 112838 [details] [review] [PATCH 2/4] bus: exit on fatal errors even if not syslogging fixed in git for 1.9.8
Comment on attachment 112839 [details] [review] [PATCH 3/4] lcov: use builddir, not srcdir fixed in git for 1.9.8
Comment on attachment 112840 [details] [review] [PATCH 4/4] Enable subdir-objects Automake option fixed in git for 1.9.8
Created attachment 129821 [details] [review] dbus-socket-set-epoll: initialize all bytes of struct epoll_event This should be a no-op, but it shuts Valgrind up. The reason for the warning is that we fill in event.events and event.data.fd, but the union event.data actually contains more bytes than that. We'll get the same partially initialized union back from the kernel in socket_set_epoll_poll(), where we take events[i].data.fd and ignore the rest. So the current code is safe, but valgrind is right to worry.
(In reply to Philip Withnall from comment #14) > Created attachment 129821 [details] [review] [review] > dbus-socket-set-epoll: initialize all bytes of struct epoll_event > > This should be a no-op, but it shuts Valgrind up. > > The reason for the warning is that we fill in event.events and > event.data.fd, but the union event.data actually contains more bytes > than that. We'll get the same partially initialized union back from the > kernel in socket_set_epoll_poll(), where we take events[i].data.fd and > ignore the rest. So the current code is safe, but valgrind is right to > worry. This is an updated version of Simon’s patch from comment #1, which covers all the instances of epoll_event I could find. It quells the Valgrind warnings I was seeing when running test-bus as per bug #99839.
(In reply to Philip Withnall from comment #15) (In reply to Philip Withnall from comment #14) > Created attachment 129821 [details] [review] > dbus-socket-set-epoll: initialize all bytes of struct epoll_event Looks good.
I'm going to credit you as the author though - you ended up doing rather more in that commit than I did :-)
Fixed in git for 1.11.12
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.