[pulseaudio-discuss] [PATCH] iochannel: don't use variable length array in union

[Tanu Kaskinen]

On Tue, 2017-03-07 at 13:42 +0100, Georg Chini wrote:
> On 04.02.2017 13:19, Tanu Kaskinen wrote:
> > Clang didn't like the variable length array:
> > 
> > pulsecore/iochannel.c:358:17: error: fields must have a constant size:
> > 'variable length array in structure' extension will never be supported
> >          uint8_t data[CMSG_SPACE(sizeof(int) * nfd)];
> >                  ^
> > 
> > Commit 451d1d6762 introduced the variable length array in order to have
> > the correct value in msg_controllen. This patch reverts that commit and
> > uses a different way to achieve the same goal.
> > 
> > BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=99458
> > ---
> >   src/pulsecore/iochannel.c | 10 ++++++++--
> >   1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/pulsecore/iochannel.c b/src/pulsecore/iochannel.c
> > index 8ace297ff..897337522 100644
> > --- a/src/pulsecore/iochannel.c
> > +++ b/src/pulsecore/iochannel.c
> > @@ -355,7 +355,7 @@ ssize_t pa_iochannel_write_with_fds(pa_iochannel*io, const void*data, size_t l,
> >       struct iovec iov;
> >       union {
> >           struct cmsghdr hdr;
> > -        uint8_t data[CMSG_SPACE(sizeof(int) * nfd)];
> > +        uint8_t data[CMSG_SPACE(sizeof(int) * MAX_ANCIL_DATA_FDS)];
> >       } cmsg;
> >   
> >       pa_assert(io);
> > @@ -382,7 +382,13 @@ ssize_t pa_iochannel_write_with_fds(pa_iochannel*io, const void*data, size_t l,
> >       mh.msg_iov = &iov;
> >       mh.msg_iovlen = 1;
> >       mh.msg_control = &cmsg;
> > -    mh.msg_controllen = sizeof(cmsg);
> > +
> > +    /* If we followed the example on the cmsg man page, we'd use
> > +     * sizeof(cmsg.data) here, but if nfd < MAX_ANCIL_DATA_FDS, then the data
> > +     * buffer is larger than needed, and the kernel doesn't like it if we set
> > +     * msg_controllen to a larger than necessary value. The commit message for
> > +     * commit 451d1d6762 contains a longer explanation. */
> > +    mh.msg_controllen = CMSG_SPACE(sizeof(int) * nfd);
> >   
> >       if ((r = sendmsg(io->ofd, &mh, MSG_NOSIGNAL)) >= 0) {
> >           io->writable = io->hungup = false;
> 
> Looks OK to me. Someone is complaining on IRC about that bug, so maybe
> you should push it soon.

Thanks for the review, I applied the patch now.

That someone is probably the Travis bot. Travis does a test build every
time something is pushed, and that started to fail when a clang build
was added to the Travis configuration.

-- 
Tanu

https://www.patreon.com/tanuk