On Sun, 2013-04-07 at 11:59 +0200, David Henningsson wrote:
> On 04/05/2013 09:44 PM, David Henningsson wrote:
> > During a stream, most packets sent are either memblocks (with SHM info),
> > or requests for more data. These are only slightly bigger than the
> > header.
> >
> > This patch makes it possible to read these packages in one call to do_read,
> > thus saving us an extra poll syscall.
> >
> > Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> > ---
> > src/pulsecore/pstream.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/src/pulsecore/pstream.c b/src/pulsecore/pstream.c
> > index d92178f..c6f1302 100644
> > --- a/src/pulsecore/pstream.c
> > +++ b/src/pulsecore/pstream.c
> > @@ -27,6 +27,7 @@
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <unistd.h>
> > +#include <errno.h>
> >
> > #ifdef HAVE_NETINET_IN_H
> > #include <netinet/in.h>
> > @@ -652,6 +653,7 @@ static int do_read(pa_pstream *p) {
> > pa_assert(p);
> > pa_assert(PA_REFCNT_VALUE(p) > 0);
> >
> > +again:
> > if (p->read.index < PA_PSTREAM_DESCRIPTOR_SIZE) {
> > d = (uint8_t*) p->read.descriptor + p->read.index;
> > l = PA_PSTREAM_DESCRIPTOR_SIZE - p->read.index;
> > @@ -774,6 +776,10 @@ static int do_read(pa_pstream *p) {
> > }
> > }
> >
> > + /* Optimisation: See if we can read the payload too */
> > + if ((p->read.data || p->read.memblock) && (r == PA_PSTREAM_DESCRIPTOR_SIZE))
> > + goto again;
> > +
> > } else if (p->read.index > PA_PSTREAM_DESCRIPTOR_SIZE) {
> > /* Frame payload available */
> >
> > @@ -894,6 +900,8 @@ fail:
>
> It's probably better to do:
>
> if (r == -1 && (errno == EAGAIN || errno == EWOULDBLOCK))
> r = 0;
> else
> r = -1;
>
> > if (release_memblock)
> > pa_memblock_release(release_memblock);
> >
>
> return r;
>
> That way we don't have to make the assumption that pa_memblock_release
> can't destroy errno.
Yes.
I also have some reservations about using goto to backwards in a complex
function. I'm not sure, however, that using a proper loop structure
would make the code any more readable. Splitting the function into
smaller functions is something to consider, but I'm not demanding that
if you don't think it's a good idea.
--
Tanu
